]>
Commit | Line | Data |
---|---|---|
52eaa2e5 GKH |
1 | From d7ac3c6ef5d8ce14b6381d52eb7adafdd6c8bb3c Mon Sep 17 00:00:00 2001 |
2 | From: "Gustavo A. R. Silva" <gustavo@embeddedor.com> | |
3 | Date: Wed, 9 Jan 2019 16:05:10 -0600 | |
4 | Subject: applicom: Fix potential Spectre v1 vulnerabilities | |
5 | ||
6 | From: Gustavo A. R. Silva <gustavo@embeddedor.com> | |
7 | ||
8 | commit d7ac3c6ef5d8ce14b6381d52eb7adafdd6c8bb3c upstream. | |
9 | ||
10 | IndexCard is indirectly controlled by user-space, hence leading to | |
11 | a potential exploitation of the Spectre variant 1 vulnerability. | |
12 | ||
13 | This issue was detected with the help of Smatch: | |
14 | ||
15 | drivers/char/applicom.c:418 ac_write() warn: potential spectre issue 'apbs' [r] | |
16 | drivers/char/applicom.c:728 ac_ioctl() warn: potential spectre issue 'apbs' [r] (local cap) | |
17 | ||
18 | Fix this by sanitizing IndexCard before using it to index apbs. | |
19 | ||
20 | Notice that given that speculation windows are large, the policy is | |
21 | to kill the speculation on the first load and not worry if it can be | |
22 | completed with a dependent load/store [1]. | |
23 | ||
24 | [1] https://lore.kernel.org/lkml/20180423164740.GY17484@dhcp22.suse.cz/ | |
25 | ||
26 | Cc: stable@vger.kernel.org | |
27 | Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> | |
28 | Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> | |
29 | ||
30 | --- | |
31 | drivers/char/applicom.c | 35 ++++++++++++++++++++++++----------- | |
32 | 1 file changed, 24 insertions(+), 11 deletions(-) | |
33 | ||
34 | --- a/drivers/char/applicom.c | |
35 | +++ b/drivers/char/applicom.c | |
36 | @@ -32,6 +32,7 @@ | |
37 | #include <linux/wait.h> | |
38 | #include <linux/init.h> | |
39 | #include <linux/fs.h> | |
40 | +#include <linux/nospec.h> | |
41 | ||
42 | #include <asm/io.h> | |
43 | #include <asm/uaccess.h> | |
44 | @@ -386,7 +387,11 @@ static ssize_t ac_write(struct file *fil | |
45 | TicCard = st_loc.tic_des_from_pc; /* tic number to send */ | |
46 | IndexCard = NumCard - 1; | |
47 | ||
48 | - if((NumCard < 1) || (NumCard > MAX_BOARD) || !apbs[IndexCard].RamIO) | |
49 | + if (IndexCard >= MAX_BOARD) | |
50 | + return -EINVAL; | |
51 | + IndexCard = array_index_nospec(IndexCard, MAX_BOARD); | |
52 | + | |
53 | + if (!apbs[IndexCard].RamIO) | |
54 | return -EINVAL; | |
55 | ||
56 | #ifdef DEBUG | |
57 | @@ -697,6 +702,7 @@ static long ac_ioctl(struct file *file, | |
58 | unsigned char IndexCard; | |
59 | void __iomem *pmem; | |
60 | int ret = 0; | |
61 | + static int warncount = 10; | |
62 | volatile unsigned char byte_reset_it; | |
63 | struct st_ram_io *adgl; | |
64 | void __user *argp = (void __user *)arg; | |
65 | @@ -711,16 +717,12 @@ static long ac_ioctl(struct file *file, | |
66 | mutex_lock(&ac_mutex); | |
67 | IndexCard = adgl->num_card-1; | |
68 | ||
69 | - if(cmd != 6 && ((IndexCard >= MAX_BOARD) || !apbs[IndexCard].RamIO)) { | |
70 | - static int warncount = 10; | |
71 | - if (warncount) { | |
72 | - printk( KERN_WARNING "APPLICOM driver IOCTL, bad board number %d\n",(int)IndexCard+1); | |
73 | - warncount--; | |
74 | - } | |
75 | - kfree(adgl); | |
76 | - mutex_unlock(&ac_mutex); | |
77 | - return -EINVAL; | |
78 | - } | |
79 | + if (cmd != 6 && IndexCard >= MAX_BOARD) | |
80 | + goto err; | |
81 | + IndexCard = array_index_nospec(IndexCard, MAX_BOARD); | |
82 | + | |
83 | + if (cmd != 6 && !apbs[IndexCard].RamIO) | |
84 | + goto err; | |
85 | ||
86 | switch (cmd) { | |
87 | ||
88 | @@ -838,5 +840,16 @@ static long ac_ioctl(struct file *file, | |
89 | kfree(adgl); | |
90 | mutex_unlock(&ac_mutex); | |
91 | return 0; | |
92 | + | |
93 | +err: | |
94 | + if (warncount) { | |
95 | + pr_warn("APPLICOM driver IOCTL, bad board number %d\n", | |
96 | + (int)IndexCard + 1); | |
97 | + warncount--; | |
98 | + } | |
99 | + kfree(adgl); | |
100 | + mutex_unlock(&ac_mutex); | |
101 | + return -EINVAL; | |
102 | + | |
103 | } | |
104 |