[PATCH] drm/ioc32: stop speculation on the drm_compat_ioctl path

[PATCH] drm/ioc32: stop speculation on the drm_compat_ioctl path

[Greg Kroah-Hartman]

The drm compat ioctl path takes a user controlled pointer, and then
dereferences it into a table of function pointers, the signature method
of spectre problems.  Fix this up by calling array_index_nospec() on the
index to the function pointer list.

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: David Airlie <airlied@gmail.com>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: stable <stable@kernel.org>
Assisted-by: gkh_clanker_2000
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
My scripts caught this codepath as not being "protected" for the
old-school spectre attack.  I don't know how realistic it is, but it
seems like this is the correct thing to be doing for a 32bit ioctl on
the drm path, as "local" users can make these.

 drivers/gpu/drm/drm_ioc32.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/drm_ioc32.c b/drivers/gpu/drm/drm_ioc32.c
index e6b5b06de148..f3e40d1e6098 100644
--- a/drivers/gpu/drm/drm_ioc32.c
+++ b/drivers/gpu/drm/drm_ioc32.c
@@ -28,6 +28,7 @@
  * IN THE SOFTWARE.
  */
 #include <linux/compat.h>
+#include <linux/nospec.h>
 #include <linux/ratelimit.h>
 #include <linux/export.h>
 
@@ -374,6 +375,7 @@ long drm_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 	if (nr >= ARRAY_SIZE(drm_compat_ioctls))
 		return drm_ioctl(filp, cmd, arg);
 
+	nr = array_index_nospec(nr, ARRAY_SIZE(drm_compat_ioctls));
 	fn = drm_compat_ioctls[nr].fn;
 	if (!fn)
 		return drm_ioctl(filp, cmd, arg);
-- 
2.53.0

Re: [PATCH] drm/ioc32: stop speculation on the drm_compat_ioctl path

[Thomas Zimmermann]

Am 24.03.26 um 17:42 schrieb Greg Kroah-Hartman:
> The drm compat ioctl path takes a user controlled pointer, and then
> dereferences it into a table of function pointers, the signature method
> of spectre problems.  Fix this up by calling array_index_nospec() on the
> index to the function pointer list.
>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: stable <stable@kernel.org>
> Assisted-by: gkh_clanker_2000
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> My scripts caught this codepath as not being "protected" for the
> old-school spectre attack.  I don't know how realistic it is, but it
> seems like this is the correct thing to be doing for a 32bit ioctl on
> the drm path, as "local" users can make these.

Seems reasonable.

Acked-by: Thomas Zimmermann <tzimmermann@suse.de>

>
>   drivers/gpu/drm/drm_ioc32.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_ioc32.c b/drivers/gpu/drm/drm_ioc32.c
> index e6b5b06de148..f3e40d1e6098 100644
> --- a/drivers/gpu/drm/drm_ioc32.c
> +++ b/drivers/gpu/drm/drm_ioc32.c
> @@ -28,6 +28,7 @@
>    * IN THE SOFTWARE.
>    */
>   #include <linux/compat.h>
> +#include <linux/nospec.h>
>   #include <linux/ratelimit.h>
>   #include <linux/export.h>
>   
> @@ -374,6 +375,7 @@ long drm_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>   	if (nr >= ARRAY_SIZE(drm_compat_ioctls))
>   		return drm_ioctl(filp, cmd, arg);
>   
> +	nr = array_index_nospec(nr, ARRAY_SIZE(drm_compat_ioctls));
>   	fn = drm_compat_ioctls[nr].fn;
>   	if (!fn)
>   		return drm_ioctl(filp, cmd, arg);

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)