| Message ID | 20191204162803.3841140-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/gem: Hook user-extensions upto MMAP_OFFSET_IOCTL | expand |
On Wed, 4 Dec 2019 at 16:29, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > From: Chris Wilson <ickle@braswell.alporthouse.com> > > Call i915_user_extensions() to validate the arg->extensions pointer, and > so return consistent error numbers for the future. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Abdiel Janulgue <abdiel.janulgue@linux.intel.com> > Cc: Matthew Auld <matthew.auld@intel.com> > --- > drivers/gpu/drm/i915/gem/i915_gem_mman.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c > index 37aabbfa869a..3a3f30bc8ac7 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c > @@ -17,6 +17,7 @@ > #include "i915_gem_object.h" > #include "i915_gem_mman.h" > #include "i915_trace.h" > +#include "i915_user_extensions.h" > #include "i915_vma.h" > > static inline bool > @@ -617,9 +618,12 @@ i915_gem_mmap_offset_ioctl(struct drm_device *dev, void *data, > struct drm_i915_private *i915 = to_i915(dev); > struct drm_i915_gem_mmap_offset *args = data; > enum i915_mmap_type type; > + int err; > > - if (args->extensions) > - return -EINVAL; > + err = i915_user_extensions(u64_to_user_ptr(args->extensions), > + NULL, 0, NULL); > + if (err) > + return err; Don't we have to check get_user first on the ptr? > > switch (args->flags) { > case I915_MMAP_OFFSET_GTT: > -- > 2.24.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoting Matthew Auld (2019-12-04 16:52:12) > On Wed, 4 Dec 2019 at 16:29, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > > From: Chris Wilson <ickle@braswell.alporthouse.com> > > > > Call i915_user_extensions() to validate the arg->extensions pointer, and > > so return consistent error numbers for the future. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Abdiel Janulgue <abdiel.janulgue@linux.intel.com> > > Cc: Matthew Auld <matthew.auld@intel.com> > > --- > > drivers/gpu/drm/i915/gem/i915_gem_mman.c | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c > > index 37aabbfa869a..3a3f30bc8ac7 100644 > > --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c > > @@ -17,6 +17,7 @@ > > #include "i915_gem_object.h" > > #include "i915_gem_mman.h" > > #include "i915_trace.h" > > +#include "i915_user_extensions.h" > > #include "i915_vma.h" > > > > static inline bool > > @@ -617,9 +618,12 @@ i915_gem_mmap_offset_ioctl(struct drm_device *dev, void *data, > > struct drm_i915_private *i915 = to_i915(dev); > > struct drm_i915_gem_mmap_offset *args = data; > > enum i915_mmap_type type; > > + int err; > > > > - if (args->extensions) > > - return -EINVAL; > > + err = i915_user_extensions(u64_to_user_ptr(args->extensions), > > + NULL, 0, NULL); > > + if (err) > > + return err; > > Don't we have to check get_user first on the ptr? No, it's checked by i915_user_extensions(). It's a bit redundant in the other callers :) Since the first thing we do is while (ext). I have some memory of starting out with NULL being an error for i915_user_extensions, but we settled on while(ext) for a compact non-recursive loop. -Chris
On Wed, 4 Dec 2019 at 17:24, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > Quoting Matthew Auld (2019-12-04 16:52:12) > > On Wed, 4 Dec 2019 at 16:29, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > > > > From: Chris Wilson <ickle@braswell.alporthouse.com> > > > > > > Call i915_user_extensions() to validate the arg->extensions pointer, and > > > so return consistent error numbers for the future. > > > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > > Cc: Abdiel Janulgue <abdiel.janulgue@linux.intel.com> > > > Cc: Matthew Auld <matthew.auld@intel.com> > > > --- > > > drivers/gpu/drm/i915/gem/i915_gem_mman.c | 8 ++++++-- > > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c > > > index 37aabbfa869a..3a3f30bc8ac7 100644 > > > --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c > > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c > > > @@ -17,6 +17,7 @@ > > > #include "i915_gem_object.h" > > > #include "i915_gem_mman.h" > > > #include "i915_trace.h" > > > +#include "i915_user_extensions.h" > > > #include "i915_vma.h" > > > > > > static inline bool > > > @@ -617,9 +618,12 @@ i915_gem_mmap_offset_ioctl(struct drm_device *dev, void *data, > > > struct drm_i915_private *i915 = to_i915(dev); > > > struct drm_i915_gem_mmap_offset *args = data; > > > enum i915_mmap_type type; > > > + int err; > > > > > > - if (args->extensions) > > > - return -EINVAL; > > > + err = i915_user_extensions(u64_to_user_ptr(args->extensions), > > > + NULL, 0, NULL); > > > + if (err) > > > + return err; > > > > Don't we have to check get_user first on the ptr? > > No, it's checked by i915_user_extensions(). It's a bit redundant in the > other callers :) Since the first thing we do is while (ext). > > I have some memory of starting out with NULL being an error for > i915_user_extensions, but we settled on while(ext) for a compact > non-recursive loop. Reviewed-by: Matthew Auld <matthew.auld@intel.com> > -Chris
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c index 37aabbfa869a..3a3f30bc8ac7 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c @@ -17,6 +17,7 @@ #include "i915_gem_object.h" #include "i915_gem_mman.h" #include "i915_trace.h" +#include "i915_user_extensions.h" #include "i915_vma.h" static inline bool @@ -617,9 +618,12 @@ i915_gem_mmap_offset_ioctl(struct drm_device *dev, void *data, struct drm_i915_private *i915 = to_i915(dev); struct drm_i915_gem_mmap_offset *args = data; enum i915_mmap_type type; + int err; - if (args->extensions) - return -EINVAL; + err = i915_user_extensions(u64_to_user_ptr(args->extensions), + NULL, 0, NULL); + if (err) + return err; switch (args->flags) { case I915_MMAP_OFFSET_GTT: