On 08/20/2018 10:53 PM, Deepak Singh Rawat wrote:
Looks good to me based on my limited understanding. Thomas/Sinclair can
could you please review and then we can include this in drm-fixes.
Thanks,
Deepak
arg.version 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/vmwgfx/vmwgfx_execbuf.c:4526 vmw_execbuf_ioctl()
warn:
potential spectre issue 'copy_offset' [w]
Fix this by sanitizing arg.version before using it to index copy_offset
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]
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Gustavo A. R. Silva <gustavo@xxxxxxxxxxxxxx>
---
drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
index 1f13457..ad91c6e 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
@@ -25,6 +25,7 @@
*
**********************************************************
****************/
#include <linux/sync_file.h>
+#include <linux/nospec.h>
#include "vmwgfx_drv.h"
#include "vmwgfx_reg.h"
@@ -4520,8 +4521,10 @@ int vmw_execbuf_ioctl(struct drm_device *dev,
unsigned long data,
return -EINVAL;
}
- if (arg.version > 1 &&
- copy_from_user(&arg.context_handle,
+ if (arg.version >= ARRAY_SIZE(copy_offset))
+ return -EFAULT;
I must admit my understanding of spectre workings in this case is
limited, but why do you need to compare
arg.version against ARRAY_SIZE here, when it is already checked against
DRM_VMW_EXECBUF_VERSION earlier?
+ arg.version = array_index_nospec(arg.version,
ARRAY_SIZE(copy_offset));
+ if (copy_from_user(&arg.context_handle,
(void __user *) (data + copy_offset[0]),
copy_offset[arg.version - 1] -
copy_offset[0]) != 0)
Similarly, we want to perform this copy iff arg.version > 1. Why did you
remove that check?
Thanks,
Thomas
--
2.7.4
_______________________________________________
Sent to linux-graphics-maintainer@xxxxxxxxxx
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
