Skip to content

Commit d913d32

Browse files
q2venkuba-moo
authored andcommitted
netlink: Use copy_to_user() for optval in netlink_getsockopt().
Brad Spencer provided a detailed report [0] that when calling getsockopt() for AF_NETLINK, some SOL_NETLINK options set only 1 byte even though such options require at least sizeof(int) as length. The options return a flag value that fits into 1 byte, but such behaviour confuses users who do not initialise the variable before calling getsockopt() and do not strictly check the returned value as char. Currently, netlink_getsockopt() uses put_user() to copy data to optlen and optval, but put_user() casts the data based on the pointer, char *optval. As a result, only 1 byte is set to optval. To avoid this behaviour, we need to use copy_to_user() or cast optval for put_user(). Note that this changes the behaviour on big-endian systems, but we document that the size of optval is int in the man page. $ man 7 netlink ... Socket options To set or get a netlink socket option, call getsockopt(2) to read or setsockopt(2) to write the option with the option level argument set to SOL_NETLINK. Unless otherwise noted, optval is a pointer to an int. Fixes: 9a4595b ("[NETLINK]: Add set/getsockopt options to support more than 32 groups") Fixes: be0c22a ("netlink: add NETLINK_BROADCAST_ERROR socket option") Fixes: 38938bf ("netlink: add NETLINK_NO_ENOBUFS socket flag") Fixes: 0a6a3a2 ("netlink: add NETLINK_CAP_ACK socket option") Fixes: 2d4bc93 ("netlink: extended ACK reporting") Fixes: 89d3552 ("netlink: Add new socket option to enable strict checking on dumps") Reported-by: Brad Spencer <[email protected]> Link: https://lore.kernel.org/netdev/[email protected]/ Signed-off-by: Kuniyuki Iwashima <[email protected]> Reviewed-by: Johannes Berg <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent 60fd497 commit d913d32

File tree

1 file changed

+23
-52
lines changed

1 file changed

+23
-52
lines changed

net/netlink/af_netlink.c

Lines changed: 23 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1742,7 +1742,8 @@ static int netlink_getsockopt(struct socket *sock, int level, int optname,
17421742
{
17431743
struct sock *sk = sock->sk;
17441744
struct netlink_sock *nlk = nlk_sk(sk);
1745-
int len, val, err;
1745+
unsigned int flag;
1746+
int len, val;
17461747

17471748
if (level != SOL_NETLINK)
17481749
return -ENOPROTOOPT;
@@ -1754,39 +1755,17 @@ static int netlink_getsockopt(struct socket *sock, int level, int optname,
17541755

17551756
switch (optname) {
17561757
case NETLINK_PKTINFO:
1757-
if (len < sizeof(int))
1758-
return -EINVAL;
1759-
len = sizeof(int);
1760-
val = nlk->flags & NETLINK_F_RECV_PKTINFO ? 1 : 0;
1761-
if (put_user(len, optlen) ||
1762-
put_user(val, optval))
1763-
return -EFAULT;
1764-
err = 0;
1758+
flag = NETLINK_F_RECV_PKTINFO;
17651759
break;
17661760
case NETLINK_BROADCAST_ERROR:
1767-
if (len < sizeof(int))
1768-
return -EINVAL;
1769-
len = sizeof(int);
1770-
val = nlk->flags & NETLINK_F_BROADCAST_SEND_ERROR ? 1 : 0;
1771-
if (put_user(len, optlen) ||
1772-
put_user(val, optval))
1773-
return -EFAULT;
1774-
err = 0;
1761+
flag = NETLINK_F_BROADCAST_SEND_ERROR;
17751762
break;
17761763
case NETLINK_NO_ENOBUFS:
1777-
if (len < sizeof(int))
1778-
return -EINVAL;
1779-
len = sizeof(int);
1780-
val = nlk->flags & NETLINK_F_RECV_NO_ENOBUFS ? 1 : 0;
1781-
if (put_user(len, optlen) ||
1782-
put_user(val, optval))
1783-
return -EFAULT;
1784-
err = 0;
1764+
flag = NETLINK_F_RECV_NO_ENOBUFS;
17851765
break;
17861766
case NETLINK_LIST_MEMBERSHIPS: {
1787-
int pos, idx, shift;
1767+
int pos, idx, shift, err = 0;
17881768

1789-
err = 0;
17901769
netlink_lock_table();
17911770
for (pos = 0; pos * 8 < nlk->ngroups; pos += sizeof(u32)) {
17921771
if (len - pos < sizeof(u32))
@@ -1803,40 +1782,32 @@ static int netlink_getsockopt(struct socket *sock, int level, int optname,
18031782
if (put_user(ALIGN(nlk->ngroups / 8, sizeof(u32)), optlen))
18041783
err = -EFAULT;
18051784
netlink_unlock_table();
1806-
break;
1785+
return err;
18071786
}
18081787
case NETLINK_CAP_ACK:
1809-
if (len < sizeof(int))
1810-
return -EINVAL;
1811-
len = sizeof(int);
1812-
val = nlk->flags & NETLINK_F_CAP_ACK ? 1 : 0;
1813-
if (put_user(len, optlen) ||
1814-
put_user(val, optval))
1815-
return -EFAULT;
1816-
err = 0;
1788+
flag = NETLINK_F_CAP_ACK;
18171789
break;
18181790
case NETLINK_EXT_ACK:
1819-
if (len < sizeof(int))
1820-
return -EINVAL;
1821-
len = sizeof(int);
1822-
val = nlk->flags & NETLINK_F_EXT_ACK ? 1 : 0;
1823-
if (put_user(len, optlen) || put_user(val, optval))
1824-
return -EFAULT;
1825-
err = 0;
1791+
flag = NETLINK_F_EXT_ACK;
18261792
break;
18271793
case NETLINK_GET_STRICT_CHK:
1828-
if (len < sizeof(int))
1829-
return -EINVAL;
1830-
len = sizeof(int);
1831-
val = nlk->flags & NETLINK_F_STRICT_CHK ? 1 : 0;
1832-
if (put_user(len, optlen) || put_user(val, optval))
1833-
return -EFAULT;
1834-
err = 0;
1794+
flag = NETLINK_F_STRICT_CHK;
18351795
break;
18361796
default:
1837-
err = -ENOPROTOOPT;
1797+
return -ENOPROTOOPT;
18381798
}
1839-
return err;
1799+
1800+
if (len < sizeof(int))
1801+
return -EINVAL;
1802+
1803+
len = sizeof(int);
1804+
val = nlk->flags & flag ? 1 : 0;
1805+
1806+
if (put_user(len, optlen) ||
1807+
copy_to_user(optval, &val, len))
1808+
return -EFAULT;
1809+
1810+
return 0;
18401811
}
18411812

18421813
static void netlink_cmsg_recv_pktinfo(struct msghdr *msg, struct sk_buff *skb)

0 commit comments

Comments
 (0)