Messages in this thread Patch in this message | | | From | Wenwen Wang <> | Subject | [PATCH] net: socket: fix a missing-check bug | Date | Sat, 20 Oct 2018 10:58:10 -0500 |
| |
In ethtool_ioctl(), the ioctl command is firstly obtained from the user-space buffer 'compat_rxnfc' through get_user() and saved to 'ethcmd'. Then, 'ethcmd' is checked to see whether it is necessary to pre-process the ethool structure, because the structure ethtool_rxnfc is defined with padding, as mentioned in the comment. If yes, a user-space buffer 'rxnfc' is allocated through compat_alloc_user_space() and then the data in the original buffer 'compat_rxnfc' is copied to 'rxnfc' through copy_in_user(), including the ioctl command. It is worth noting that after this copy, there is no check enforced on the copied ioctl command. That means it is possible that 'rxnfc->cmd' is different from 'ethcmd', because a malicious user can race to modify the ioctl command in 'compat_rxnfc' between these two copies. Eventually, the ioctl command in 'rxnfc' will be used in dev_ethtool(). This can cause undefined behavior of the kernel and introduce potential security risk.
This patch avoids the above issue by rewriting 'rxnfc->cmd' using 'ethcmd' after copy_in_user().
Signed-off-by: Wenwen Wang <wang6495@umn.edu> --- net/socket.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/net/socket.c b/net/socket.c index 01f3f8f..c5f969c 100644 --- a/net/socket.c +++ b/net/socket.c @@ -2879,6 +2879,8 @@ static int ethtool_ioctl(struct net *net, struct compat_ifreq __user *ifr32) copy_in_user(&rxnfc->rule_cnt, &compat_rxnfc->rule_cnt, sizeof(rxnfc->rule_cnt))) return -EFAULT; + + rxnfc->cmd = ethcmd; } ret = dev_ioctl(net, SIOCETHTOOL, &ifr, NULL); -- 2.7.4
| |