drm/ioctl: Fix Spectre v1 vulnerabilities
Subsystems (2)
PATCH 1/1 drm/ioctl: Fix Spectre v1 vulnerabilities
nr is 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/gpu/drm/drm_ioctl.c:805 drm_ioctl() warn: potential spectre issue 'dev->driver->ioctls' [r] drivers/gpu/drm/drm_ioctl.c:810 drm_ioctl() warn: potential spectre issue 'drm_ioctls' [r] (local cap) drivers/gpu/drm/drm_ioctl.c:892 drm_ioctl_flags() warn: potential spectre issue 'drm_ioctls' [r] (local cap)
Fix this by sanitizing nr before using it to index dev->driver->ioctls and drm_ioctls.
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
--- drivers/gpu/drm/drm_ioctl.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
drivers/gpu/drm/drm_ioctl.c +8 -2
@@ -37,6 +37,7 @@#include <linux/pci.h> #include <linux/export.h> +#include <linux/nospec.h> /** * DOC: getunique and setversion story
@@ -800,13 +801,17 @@ long drm_ioctl(struct file *filp,if (is_driver_ioctl) {/* driver ioctl */ -if (nr -DRM_COMMAND_BASE >= dev->driver->num_ioctls) +unsigned int index = nr -DRM_COMMAND_BASE ; + +if (index >= dev->driver->num_ioctls) goto err_i1; - ioctl = &dev->driver->ioctls[nr -DRM_COMMAND_BASE ]; + index =array_index_nospec (index, dev->driver->num_ioctls); + ioctl = &dev->driver->ioctls[index]; }else {/* core ioctl */ if (nr >=DRM_CORE_IOCTL_COUNT ) goto err_i1; + nr =array_index_nospec (nr,DRM_CORE_IOCTL_COUNT ); ioctl = &drm_ioctls[nr]; }
@@ -888,6 +893,7 @@ bool drm_ioctl_flags(unsigned int nr, unsigned int *flags)if (nr >=DRM_CORE_IOCTL_COUNT )return false; + nr =array_index_nospec (nr,DRM_CORE_IOCTL_COUNT ); *flags = drm_ioctls[nr].flags;return true;
Signature · 1 line
2.20.1
Quoted 12 lines · Gustavo A. R. Silva
nr is 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/gpu/drm/drm_ioctl.c:805drm_ioctl()warn: potential spectre issue 'dev->driver->ioctls' [r]drivers/gpu/drm/drm_ioctl.c:810drm_ioctl()warn: potential spectre issue 'drm_ioctls' [r] (local cap)drivers/gpu/drm/drm_ioctl.c:892drm_ioctl_flags()warn: potential spectre issue 'drm_ioctls' [r] (local cap)Fix this by sanitizing nr before using it to index
dev->driver->ioctlsanddrm_ioctls.
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
Signed-off-by Gustavo A. R. Silva <[email protected]>
lgtm and I think there's no other obvious place where we need array_index_nospec in drm core. Applied to drm-misc-fixes. -Daniel
Quoted 47 lines
--- drivers/gpu/drm/drm_ioctl.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)drivers/gpu/drm/drm_ioctl.c +8
-2@@ -37,6 +37,7 @@#include <linux/pci.h> #include <linux/export.h> +#include <linux/nospec.h> /** * DOC: getunique and setversion story @@ -800,13 +801,17 @@ long drm_ioctl(struct file *filp,if (is_driver_ioctl) {/* driver ioctl */ -if (nr -DRM_COMMAND_BASE >= dev->driver->num_ioctls) +unsigned int index = nr -DRM_COMMAND_BASE ; + +if (index >= dev->driver->num_ioctls) goto err_i1; - ioctl = &dev->driver->ioctls[nr -DRM_COMMAND_BASE ]; + index =array_index_nospec (index, dev->driver->num_ioctls); + ioctl = &dev->driver->ioctls[index]; }else {/* core ioctl */ if (nr >=DRM_CORE_IOCTL_COUNT ) goto err_i1; + nr =array_index_nospec (nr,DRM_CORE_IOCTL_COUNT ); ioctl = &drm_ioctls[nr]; }@@ -888,6 +893,7 @@ bool drm_ioctl_flags(unsigned int nr, unsigned int *flags)if (nr >=DRM_CORE_IOCTL_COUNT )return false; + nr =array_index_nospec (nr,DRM_CORE_IOCTL_COUNT ); *flags = drm_ioctls[nr].flags;return true;Signature · 1 line
2.20.1
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel