Re: [PATCH] drm/i915: do not return invalid pointers as a *dentry

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Mon, Feb 04, 2019 at 08:37:01PM +0100, Greg Kroah-Hartman wrote:
> On Mon, Feb 04, 2019 at 10:13:25AM -0800, Rodrigo Vivi wrote:
> > On Thu, Jan 31, 2019 at 07:17:02PM +0100, Greg Kroah-Hartman wrote:
> > > On Thu, Jan 31, 2019 at 09:59:26AM -0800, Rodrigo Vivi wrote:
> > > > On Thu, Jan 31, 2019 at 02:15:07PM +0100, Greg Kroah-Hartman wrote:
> > > > > When calling debugfs functions, they can now return error values if
> > > > > something went wrong.  If that happens, return a NULL as a *dentry to
> > > > > the relay core instead of passing it an illegal pointer.
> > > > > 
> > > > > The relay core should be able to handle an illegal pointer, but add this
> > > > > check to be safe.
> > > > > 
> > > > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > > > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > Cc: David Airlie <airlied@linux.ie>
> > > > > Cc: Daniel Vetter <daniel@ffwll.ch>
> > > > > Cc: intel-gfx@lists.freedesktop.org
> > > > > Cc: dri-devel@lists.freedesktop.org
> > > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/intel_guc_log.c | 3 +++
> > > > >  1 file changed, 3 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
> > > > > index d3ebdbc0182e..8bf03497dcd8 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_guc_log.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_guc_log.c
> > > > > @@ -140,6 +140,9 @@ static struct dentry *create_buf_file_callback(const char *filename,
> > > > >  
> > > > >  	buf_file = debugfs_create_file(filename, mode,
> > > > >  				       parent, buf, &relay_file_operations);
> > > > > +	if (IS_ERR(buf_file))
> > > > > +		return NULL;
> > > > 
> > > > I still see a return NULL inside debugfs_create_file on master,
> > > > but probably you are ahead with some change that I didn't see yet right?
> > > 
> > > Yes, this patch is in linux-next now and should go to Linus for
> > > 5.0-final:
> > > 	https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git/commit/?h=driver-core-linus&id=ff9fb72bc07705c00795ca48631f7fffe24d2c6b
> > > 
> > > > I'm just wondering if it wouldn't be better for now to go with
> > > > 
> > > > if (IS_ERR_OR_NULL(buf_file))
> > > 
> > > Not really, because the next line is:
> > > 
> > > > >  	return buf_file;
> > > 
> > > So it's the same thing :)
> > > 
> > > > apparently we also need it on i915_debugfs.c i915_debugfs_register()
> > > 
> > > I have a bunch of patches I'm working on to go through and fix up all of
> > > this (you shouldn't be checking the return value at all, unless you
> > > really want to use it as a "real" dentry and not just something that
> > > debugfs uses internally.
> > > 
> > > But for now, you should be fine, that call will only fail if you are out
> > > of memory, or did something really wrong.
> > 
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > 
> > do you wanna push this with your own stuff or do you want me to pick
> > up on drm-intel-next?
> 
> Which ever is easiest for you works for me, just let me know.

pushed to dinq... will be on 5.1

Thanks,
Rodrigo.

> 
> thanks,
> 
> greg k-h
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx