Skip to contentSkip to conversation

usb: gadget: storage: Fix Spectre v1 vulnerability

PATCH In review Gustavo A. R. Silva 3 participants 3 messages Oct 16, 2018 USB

lore
Share & tools
lore

Gustavo A. R. Silva

author
#0 lore raw

PATCH 1/1 usb: gadget: storage: Fix Spectre v1 vulnerability

num can be indirectly 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/usb/gadget/function/f_mass_storage.c:3177 fsg_lun_make() warn:
potential spectre issue 'fsg_opts->common->luns' [r] (local cap)

Fix this by sanitizing num before using it to index fsg_opts->common->luns

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://marc.info/?l=linux-kernel&m=152449131114778&w=2

Cc [email protected]
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/usb/gadget/function/f_mass_storage.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/usb/gadget/function/f_mass_storage.c +3 -0
index cb402e7a..043f97a 100644
@@ -221,6 +221,8 @@
 #include <linux/usb/gadget.h>
 #include <linux/usb/composite.h>
 
+#include <linux/nospec.h>
+
 #include "configfs.h"
 
 

@@ -3152,6 +3154,7 @@ static struct config_group *fsg_lun_make(struct config_group *group,
 	fsg_opts = to_fsg_opts(&group->cg_item);
 	if (num >= FSG_MAX_LUNS)
 		return ERR_PTR(-ERANGE);
+	num = array_index_nospec(num, FSG_MAX_LUNS);
 
 	mutex_lock(&fsg_opts->lock);
 	if (fsg_opts->refcnt || fsg_opts->common->luns[num]) {
Acked-by on v1 1/1
#1 lore raw

Hi Greg,

Quoted 11 lines · Gustavo A. R. Silva

num can be indirectly 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/usb/gadget/function/f_mass_storage.c:3177 fsg_lun_make() warn:
potential spectre issue 'fsg_opts->common->luns' [r] (local cap)

Fix this by sanitizing num before using it to index fsg_opts->common->luns

Quoted 8 lines

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://marc.info/?l=linux-kernel&m=152449131114778&w=2

Cc [email protected]
Signed-off-by Gustavo A. R. Silva <[email protected]>

Can you still take this as an urgent fix?

Acked-by Felipe Balbi <[email protected]>
maintainer
#2 lore raw
Quoted 20 lines · Felipe Balbi

Hi Greg,

Quoted 15 lines · Gustavo A. R. Silva

num can be indirectly 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/usb/gadget/function/f_mass_storage.c:3177 fsg_lun_make() warn:
potential spectre issue 'fsg_opts->common->luns' [r] (local cap)

Fix this by sanitizing num before using it to index fsg_opts->common->luns

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].

Quoted 8 lines
Quoted 4 lines

[1] https://marc.info/?l=linux-kernel&m=152449131114778&w=2

Cc [email protected]
Signed-off-by Gustavo A. R. Silva <[email protected]>

Can you still take this as an urgent fix?

Acked-by Felipe Balbi <[email protected]>

Yes, will do so, thanks.

greg k-h

Quoted 3 lines
Signature · 2 lines
-- 
balbi