On Thu, Mar 14, 2013 at 12:59:57PM +0000, Chris Wilson wrote:
In order to prevent a potential NULL deference with hostile userspace, we need to check whether the ioctl was passed an invalid args pointer.
Reported-by: Tommi Rantala tt.rantala@gmail.com Link: http://lkml.kernel.org/r/CA+ydwtpuBvbwxbt-tdgPUvj1EU7itmCHo_2B3w13HkD5+jWKow... Signed-off-by: Chris Wilson chris@chris-wilson.co.uk
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 365e41a..9f5602e 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1103,7 +1103,11 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, struct drm_i915_gem_exec_object2 *exec2_list = NULL; int ret, i;
- if (args->buffer_count < 1) {
- if (args == NULL)
return -EINVAL;- if (args->buffer_count < 1 ||
args->buffer_count > INT_MAX / sizeof(*exec2_list)) {@@ -1182,8 +1186,11 @@ i915_gem_execbuffer2(struct drm_device *dev, void *data, struct drm_i915_gem_exec_object2 *exec2_list = NULL; int ret;
- if (args == NULL)
return -EINVAL;- if (args->buffer_count < 1 ||
args->buffer_count > UINT_MAX / sizeof(*exec2_list)) {
args->buffer_count > INT_MAX / sizeof(*exec2_list)) {
Why did you change UINT_MAX to INT_MAX? TBH, I'm confused what we're trying to achieve, and why we need anything other than: if (!args->buffer_count)
I'm also not seeing how the NULL checks are needed since at least it seems to be for execbuffer (IOW) we could never have NULL args.
if (cmd & (IOC_IN | IOC_OUT)) { if (asize <= sizeof(stack_kdata)) { kdata = stack_kdata; } else { kdata = kmalloc(asize, GFP_KERNEL); if (!kdata) { retcode = -ENOMEM; goto err_i1; } } if (asize > usize) memset(kdata + usize, 0, asize - usize); }
Sorry if these are stupid questions.