All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: do not return invalid pointers as a *dentry
@ 2019-01-31 13:15 Greg Kroah-Hartman
  2019-01-31 14:05 ` ✓ Fi.CI.BAT: success for " Patchwork
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Greg Kroah-Hartman @ 2019-01-31 13:15 UTC (permalink / raw)
  To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, David Airlie, Daniel Vetter
  Cc: intel-gfx, dri-devel

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;
+
 	return buf_file;
 }
 
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: do not return invalid pointers as a *dentry
  2019-01-31 13:15 [PATCH] drm/i915: do not return invalid pointers as a *dentry Greg Kroah-Hartman
@ 2019-01-31 14:05 ` Patchwork
  2019-01-31 17:59 ` [PATCH] " Rodrigo Vivi
  2019-01-31 23:17 ` ✓ Fi.CI.IGT: success for " Patchwork
  2 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2019-01-31 14:05 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: do not return invalid pointers as a *dentry
URL   : https://patchwork.freedesktop.org/series/56044/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5518 -> Patchwork_12110
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/56044/revisions/1/mbox/

Known issues
------------

  Here are the changes found in Patchwork_12110 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live_evict:
    - fi-bsw-kefka:       NOTRUN -> DMESG-WARN [fdo#107709]

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191] / [fdo#107362]

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
    - fi-apl-guc:         PASS -> DMESG-WARN [fdo#108566]

  * igt@pm_rpm@basic-rte:
    - fi-byt-j1900:       PASS -> FAIL [fdo#108800]

  
#### Possible fixes ####

  * igt@i915_selftest@live_hangcheck:
    - fi-skl-iommu:       INCOMPLETE [fdo#108602] / [fdo#108744] -> PASS

  * igt@kms_busy@basic-flip-a:
    - fi-kbl-7567u:       {SKIP} [fdo#109271] / [fdo#109278] -> PASS +2
    - fi-gdg-551:         FAIL [fdo#103182] -> PASS

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       FAIL [fdo#109485] -> PASS

  * igt@kms_pipe_crc_basic@read-crc-pipe-b-frame-sequence:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS

  * igt@pm_rpm@module-reload:
    - fi-skl-6770hq:      FAIL [fdo#108511] -> PASS

  * igt@prime_vgem@basic-fence-flip:
    - fi-gdg-551:         DMESG-FAIL [fdo#103182] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107709]: https://bugs.freedesktop.org/show_bug.cgi?id=107709
  [fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108602]: https://bugs.freedesktop.org/show_bug.cgi?id=108602
  [fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744
  [fdo#108800]: https://bugs.freedesktop.org/show_bug.cgi?id=108800
  [fdo#108915]: https://bugs.freedesktop.org/show_bug.cgi?id=108915
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109485]: https://bugs.freedesktop.org/show_bug.cgi?id=109485
  [fdo#109516]: https://bugs.freedesktop.org/show_bug.cgi?id=109516


Participating hosts (43 -> 45)
------------------------------

  Additional (6): fi-bxt-dsi fi-ivb-3770 fi-icl-y fi-bsw-kefka fi-skl-6600u fi-snb-2600 
  Missing    (4): fi-bsw-cyan fi-ilk-m540 fi-byt-squawks fi-icl-u2 


Build changes
-------------

    * Linux: CI_DRM_5518 -> Patchwork_12110

  CI_DRM_5518: 2369fd28d3a46b865f6d4f1d309a4c6b7b4e6d93 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4801: 6f6bacf12759fb319ade3ba37861ae711f8a5cd9 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12110: 36920a9b0dc28118dd85e0595b13fda607b39e7e @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

36920a9b0dc2 drm/i915: do not return invalid pointers as a *dentry

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12110/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/i915: do not return invalid pointers as a *dentry
  2019-01-31 13:15 [PATCH] drm/i915: do not return invalid pointers as a *dentry Greg Kroah-Hartman
  2019-01-31 14:05 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2019-01-31 17:59 ` Rodrigo Vivi
  2019-01-31 18:17   ` Greg Kroah-Hartman
  2019-01-31 23:17 ` ✓ Fi.CI.IGT: success for " Patchwork
  2 siblings, 1 reply; 8+ messages in thread
From: Rodrigo Vivi @ 2019-01-31 17:59 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: David Airlie, intel-gfx, dri-devel

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?

I'm just wondering if it wouldn't be better for now to go with

if (IS_ERR_OR_NULL(buf_file))

apparently we also need it on i915_debugfs.c i915_debugfs_register()

Thanks,
Rodrigo.

> +
>  	return buf_file;
>  }
>  
> -- 
> 2.20.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/i915: do not return invalid pointers as a *dentry
  2019-01-31 17:59 ` [PATCH] " Rodrigo Vivi
@ 2019-01-31 18:17   ` Greg Kroah-Hartman
  2019-02-04 18:13     ` [Intel-gfx] " Rodrigo Vivi
  0 siblings, 1 reply; 8+ messages in thread
From: Greg Kroah-Hartman @ 2019-01-31 18:17 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: David Airlie, intel-gfx, dri-devel

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.

thanks,

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* ✓ Fi.CI.IGT: success for drm/i915: do not return invalid pointers as a *dentry
  2019-01-31 13:15 [PATCH] drm/i915: do not return invalid pointers as a *dentry Greg Kroah-Hartman
  2019-01-31 14:05 ` ✓ Fi.CI.BAT: success for " Patchwork
  2019-01-31 17:59 ` [PATCH] " Rodrigo Vivi
@ 2019-01-31 23:17 ` Patchwork
  2 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2019-01-31 23:17 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: do not return invalid pointers as a *dentry
URL   : https://patchwork.freedesktop.org/series/56044/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5518_full -> Patchwork_12110_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_12110_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_schedule@pi-ringfull-render:
    - shard-apl:          NOTRUN -> FAIL [fdo#103158]

  * igt@kms_color@pipe-a-legacy-gamma:
    - shard-apl:          PASS -> FAIL [fdo#104782] / [fdo#108145]

  * igt@kms_cursor_crc@cursor-256x85-random:
    - shard-apl:          PASS -> FAIL [fdo#103232]

  * igt@kms_cursor_crc@cursor-64x64-suspend:
    - shard-glk:          PASS -> FAIL [fdo#103232] +1

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-max:
    - shard-glk:          PASS -> FAIL [fdo#108145]

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-none:
    - shard-apl:          NOTRUN -> FAIL [fdo#103166]

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
    - shard-apl:          PASS -> FAIL [fdo#103166] +2

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-y:
    - shard-glk:          PASS -> FAIL [fdo#103166]

  * igt@kms_setmode@basic:
    - shard-kbl:          PASS -> FAIL [fdo#99912]

  
#### Possible fixes ####

  * igt@gem_eio@reset-stress:
    - shard-hsw:          INCOMPLETE [fdo#103540] / [fdo#109482] -> PASS

  * igt@kms_color@pipe-b-ctm-max:
    - shard-apl:          FAIL [fdo#108147] -> PASS

  * igt@kms_cursor_crc@cursor-256x256-suspend:
    - shard-hsw:          INCOMPLETE [fdo#103540] -> PASS

  * igt@kms_cursor_crc@cursor-256x85-onscreen:
    - shard-apl:          FAIL [fdo#103232] -> PASS +3

  * igt@kms_flip@flip-vs-panning-interruptible:
    - shard-hsw:          DMESG-WARN [fdo#102614] -> PASS

  * igt@kms_plane@plane-position-covered-pipe-c-planes:
    - shard-apl:          FAIL [fdo#103166] -> PASS +2

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-yf:
    - shard-glk:          FAIL [fdo#103166] -> PASS

  * igt@sw_sync@sync_busy_fork:
    - shard-snb:          INCOMPLETE [fdo#105411] -> PASS +1

  
#### Warnings ####

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b:
    - shard-snb:          {SKIP} [fdo#109271] / [fdo#109278] -> DMESG-WARN [fdo#107956]

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#103158]: https://bugs.freedesktop.org/show_bug.cgi?id=103158
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108147]: https://bugs.freedesktop.org/show_bug.cgi?id=108147
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109482]: https://bugs.freedesktop.org/show_bug.cgi?id=109482
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (7 -> 5)
------------------------------

  Missing    (2): shard-skl shard-iclb 


Build changes
-------------

    * Linux: CI_DRM_5518 -> Patchwork_12110

  CI_DRM_5518: 2369fd28d3a46b865f6d4f1d309a4c6b7b4e6d93 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4801: 6f6bacf12759fb319ade3ba37861ae711f8a5cd9 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12110: 36920a9b0dc28118dd85e0595b13fda607b39e7e @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12110/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: do not return invalid pointers as a *dentry
  2019-01-31 18:17   ` Greg Kroah-Hartman
@ 2019-02-04 18:13     ` Rodrigo Vivi
  2019-02-04 19:37       ` Greg Kroah-Hartman
  0 siblings, 1 reply; 8+ messages in thread
From: Rodrigo Vivi @ 2019-02-04 18:13 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: David Airlie, intel-gfx, dri-devel

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?

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/i915: do not return invalid pointers as a *dentry
  2019-02-04 18:13     ` [Intel-gfx] " Rodrigo Vivi
@ 2019-02-04 19:37       ` Greg Kroah-Hartman
  2019-02-05 17:48         ` Rodrigo Vivi
  0 siblings, 1 reply; 8+ messages in thread
From: Greg Kroah-Hartman @ 2019-02-04 19:37 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: David Airlie, intel-gfx, dri-devel

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.

thanks,

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/i915: do not return invalid pointers as a *dentry
  2019-02-04 19:37       ` Greg Kroah-Hartman
@ 2019-02-05 17:48         ` Rodrigo Vivi
  0 siblings, 0 replies; 8+ messages in thread
From: Rodrigo Vivi @ 2019-02-05 17:48 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: David Airlie, intel-gfx, dri-devel

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-02-05 17:48 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-31 13:15 [PATCH] drm/i915: do not return invalid pointers as a *dentry Greg Kroah-Hartman
2019-01-31 14:05 ` ✓ Fi.CI.BAT: success for " Patchwork
2019-01-31 17:59 ` [PATCH] " Rodrigo Vivi
2019-01-31 18:17   ` Greg Kroah-Hartman
2019-02-04 18:13     ` [Intel-gfx] " Rodrigo Vivi
2019-02-04 19:37       ` Greg Kroah-Hartman
2019-02-05 17:48         ` Rodrigo Vivi
2019-01-31 23:17 ` ✓ Fi.CI.IGT: success for " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.