| Message ID | 20190731032141.GA30246@embeddedor |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | atm: iphase: Fix Spectre v1 vulnerability | expand |
From: "Gustavo A. R. Silva" <gustavo@embeddedor.com> Date: Tue, 30 Jul 2019 22:21:41 -0500 > board is controlled by user-space, hence leading to a potential > exploitation of the Spectre variant 1 vulnerability. > > This issue was detected with the help of Smatch: Applied and queued up for -stable. Do not CC: -stable for networking fixes, we take care of the stable submissions manually for this subsystem. Thank you.
Hi Dave, On 8/2/19 7:31 PM, David Miller wrote: > From: "Gustavo A. R. Silva" <gustavo@embeddedor.com> > Date: Tue, 30 Jul 2019 22:21:41 -0500 > >> board is controlled by user-space, hence leading to a potential >> exploitation of the Spectre variant 1 vulnerability. >> >> This issue was detected with the help of Smatch: > > Applied and queued up for -stable. > > Do not CC: -stable for networking fixes, we take care of the stable > submissions manually for this subsystem. > Yeah. I'm aware of that. The thing is that you don't appear as a maintainer of this file: $ scripts/get_maintainer.pl --nokeywords --nogit --nogit-fallback -f drivers/atm/iphase.c Chas Williams <3chas3@gmail.com> (maintainer:ATM) linux-atm-general@lists.sourceforge.net (moderated list:ATM) netdev@vger.kernel.org (open list:ATM) linux-kernel@vger.kernel.org (open list) so, I didn't know this patch would be applied to net. Thanks -- Gustavo
diff --git a/drivers/atm/iphase.c b/drivers/atm/iphase.c index 302cf0ba1600..8c7a996d1f16 100644 --- a/drivers/atm/iphase.c +++ b/drivers/atm/iphase.c @@ -63,6 +63,7 @@ #include <asm/byteorder.h> #include <linux/vmalloc.h> #include <linux/jiffies.h> +#include <linux/nospec.h> #include "iphase.h" #include "suni.h" #define swap_byte_order(x) (((x & 0xff) << 8) | ((x & 0xff00) >> 8)) @@ -2760,8 +2761,11 @@ static int ia_ioctl(struct atm_dev *dev, unsigned int cmd, void __user *arg) } if (copy_from_user(&ia_cmds, arg, sizeof ia_cmds)) return -EFAULT; board = ia_cmds.status; - if ((board < 0) || (board > iadev_count)) - board = 0; + + if ((board < 0) || (board > iadev_count)) + board = 0; + board = array_index_nospec(board, iadev_count + 1); + iadev = ia_dev[board]; switch (ia_cmds.cmd) { case MEMDUMP:
board is controlled by user-space, hence leading to a potential exploitation of the Spectre variant 1 vulnerability. This issue was detected with the help of Smatch: drivers/atm/iphase.c:2765 ia_ioctl() warn: potential spectre issue 'ia_dev' [r] (local cap) drivers/atm/iphase.c:2774 ia_ioctl() warn: possible spectre second half. 'iadev' drivers/atm/iphase.c:2782 ia_ioctl() warn: possible spectre second half. 'iadev' drivers/atm/iphase.c:2816 ia_ioctl() warn: possible spectre second half. 'iadev' drivers/atm/iphase.c:2823 ia_ioctl() warn: possible spectre second half. 'iadev' drivers/atm/iphase.c:2830 ia_ioctl() warn: potential spectre issue '_ia_dev' [r] (local cap) drivers/atm/iphase.c:2845 ia_ioctl() warn: possible spectre second half. 'iadev' drivers/atm/iphase.c:2856 ia_ioctl() warn: possible spectre second half. 'iadev' Fix this by sanitizing board before using it to index ia_dev and _ia_dev Notice that given that speculation windows are large, the policy is to kill the speculation on the first load and not worry if it can be completed with a dependent load/store [1]. [1] https://lore.kernel.org/lkml/20180423164740.GY17484@dhcp22.suse.cz/ Cc: stable@vger.kernel.org Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> --- drivers/atm/iphase.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)