| From foo@baz Fri Nov 2 10:04:04 CET 2018 |
| From: Wenwen Wang <wang6495@umn.edu> |
| Date: Thu, 18 Oct 2018 09:36:46 -0500 |
| Subject: net: socket: fix a missing-check bug |
| |
| From: Wenwen Wang <wang6495@umn.edu> |
| |
| [ Upstream commit b6168562c8ce2bd5a30e213021650422e08764dc ] |
| |
| In ethtool_ioctl(), the ioctl command 'ethcmd' is checked through a switch |
| statement to see whether it is necessary to pre-process the ethtool |
| structure, because, as mentioned in the comment, the structure |
| ethtool_rxnfc is defined with padding. If yes, a user-space buffer 'rxnfc' |
| is allocated through compat_alloc_user_space(). One thing to note here is |
| that, if 'ethcmd' is ETHTOOL_GRXCLSRLALL, the size of the buffer 'rxnfc' is |
| partially determined by 'rule_cnt', which is actually acquired from the |
| user-space buffer 'compat_rxnfc', i.e., 'compat_rxnfc->rule_cnt', through |
| get_user(). After 'rxnfc' is allocated, the data in the original user-space |
| buffer 'compat_rxnfc' is then copied to 'rxnfc' through copy_in_user(), |
| including the 'rule_cnt' field. However, after this copy, no check is |
| re-enforced on 'rxnfc->rule_cnt'. So it is possible that a malicious user |
| race to change the value in the 'compat_rxnfc->rule_cnt' between these two |
| copies. Through this way, the attacker can bypass the previous check on |
| 'rule_cnt' and inject malicious data. This can cause undefined behavior of |
| the kernel and introduce potential security risk. |
| |
| This patch avoids the above issue via copying the value acquired by |
| get_user() to 'rxnfc->rule_cn', if 'ethcmd' is ETHTOOL_GRXCLSRLALL. |
| |
| Signed-off-by: Wenwen Wang <wang6495@umn.edu> |
| Signed-off-by: David S. Miller <davem@davemloft.net> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| net/socket.c | 11 ++++++++--- |
| 1 file changed, 8 insertions(+), 3 deletions(-) |
| |
| --- a/net/socket.c |
| +++ b/net/socket.c |
| @@ -2774,9 +2774,14 @@ static int ethtool_ioctl(struct net *net |
| copy_in_user(&rxnfc->fs.ring_cookie, |
| &compat_rxnfc->fs.ring_cookie, |
| (void __user *)(&rxnfc->fs.location + 1) - |
| - (void __user *)&rxnfc->fs.ring_cookie) || |
| - copy_in_user(&rxnfc->rule_cnt, &compat_rxnfc->rule_cnt, |
| - sizeof(rxnfc->rule_cnt))) |
| + (void __user *)&rxnfc->fs.ring_cookie)) |
| + return -EFAULT; |
| + if (ethcmd == ETHTOOL_GRXCLSRLALL) { |
| + if (put_user(rule_cnt, &rxnfc->rule_cnt)) |
| + return -EFAULT; |
| + } else if (copy_in_user(&rxnfc->rule_cnt, |
| + &compat_rxnfc->rule_cnt, |
| + sizeof(rxnfc->rule_cnt))) |
| return -EFAULT; |
| } |
| |