| From foo@baz Sun 09 Jun 2019 09:24:16 AM CEST |
| From: Vivien Didelot <vivien.didelot@gmail.com> |
| Date: Mon, 3 Jun 2019 16:57:13 -0400 |
| Subject: ethtool: fix potential userspace buffer overflow |
| |
| From: Vivien Didelot <vivien.didelot@gmail.com> |
| |
| [ Upstream commit 0ee4e76937d69128a6a66861ba393ebdc2ffc8a2 ] |
| |
| ethtool_get_regs() allocates a buffer of size ops->get_regs_len(), |
| and pass it to the kernel driver via ops->get_regs() for filling. |
| |
| There is no restriction about what the kernel drivers can or cannot do |
| with the open ethtool_regs structure. They usually set regs->version |
| and ignore regs->len or set it to the same size as ops->get_regs_len(). |
| |
| But if userspace allocates a smaller buffer for the registers dump, |
| we would cause a userspace buffer overflow in the final copy_to_user() |
| call, which uses the regs.len value potentially reset by the driver. |
| |
| To fix this, make this case obvious and store regs.len before calling |
| ops->get_regs(), to only copy as much data as requested by userspace, |
| up to the value returned by ops->get_regs_len(). |
| |
| While at it, remove the redundant check for non-null regbuf. |
| |
| Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com> |
| Reviewed-by: Michal Kubecek <mkubecek@suse.cz> |
| Signed-off-by: David S. Miller <davem@davemloft.net> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| net/core/ethtool.c | 5 ++++- |
| 1 file changed, 4 insertions(+), 1 deletion(-) |
| |
| --- a/net/core/ethtool.c |
| +++ b/net/core/ethtool.c |
| @@ -1358,13 +1358,16 @@ static int ethtool_get_regs(struct net_d |
| if (!regbuf) |
| return -ENOMEM; |
| |
| + if (regs.len < reglen) |
| + reglen = regs.len; |
| + |
| ops->get_regs(dev, ®s, regbuf); |
| |
| ret = -EFAULT; |
| if (copy_to_user(useraddr, ®s, sizeof(regs))) |
| goto out; |
| useraddr += offsetof(struct ethtool_regs, data); |
| - if (regbuf && copy_to_user(useraddr, regbuf, regs.len)) |
| + if (copy_to_user(useraddr, regbuf, reglen)) |
| goto out; |
| ret = 0; |
| |