[PATCH] drm/i915: fix driver's versions of WARN_ON & WARN_ON_ONCE

[PATCH] drm/i915: fix driver's versions of WARN_ON & WARN_ON_ONCE

[Dave Gordon]

The current versions of these two macros don't work correctly if the
argument expression happens to contain a modulo operator (%) -- when
stringified, it gets interpreted as a printf formatting character!
With a specifically crafted parameter, this could probably cause a
kernel OOPS; consider WARN_ON(p%s) or WARN_ON(f %*pEp).

Instead, we should use an explicit "%s" format, with the stringified
expression as the coresponding literal-string argument.

Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b157865..674b223 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -67,11 +67,11 @@
 		BUILD_BUG_ON(__i915_warn_cond); \
 	WARN(__i915_warn_cond, "WARN_ON(" #x ")"); })
 #else
-#define WARN_ON(x) WARN((x), "WARN_ON(" #x ")")
+#define WARN_ON(x) WARN((x), "WARN_ON(%s)", #x )
 #endif
 
 #undef WARN_ON_ONCE
-#define WARN_ON_ONCE(x) WARN_ONCE((x), "WARN_ON_ONCE(" #x ")")
+#define WARN_ON_ONCE(x) WARN_ONCE((x), "WARN_ON_ONCE(%s)", #x )
 
 #define MISSING_CASE(x) WARN(1, "Missing switch case (%lu) in %s\n", \
 			     (long) (x), __func__);
-- 
1.9.1

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

Re: [PATCH] drm/i915: fix driver's versions of WARN_ON & WARN_ON_ONCE

[Paulo Zanoni]

2015-08-17 13:30 GMT-03:00 Dave Gordon <david.s.gordon@intel.com>:
> The current versions of these two macros don't work correctly if the
> argument expression happens to contain a modulo operator (%) -- when
> stringified, it gets interpreted as a printf formatting character!
> With a specifically crafted parameter, this could probably cause a
> kernel OOPS; consider WARN_ON(p%s) or WARN_ON(f %*pEp).
>
> Instead, we should use an explicit "%s" format, with the stringified
> expression as the coresponding literal-string argument.

I suppose that was inspired by the recent IGT commit :)

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

>
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b157865..674b223 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -67,11 +67,11 @@
>                 BUILD_BUG_ON(__i915_warn_cond); \
>         WARN(__i915_warn_cond, "WARN_ON(" #x ")"); })
>  #else
> -#define WARN_ON(x) WARN((x), "WARN_ON(" #x ")")
> +#define WARN_ON(x) WARN((x), "WARN_ON(%s)", #x )
>  #endif
>
>  #undef WARN_ON_ONCE
> -#define WARN_ON_ONCE(x) WARN_ONCE((x), "WARN_ON_ONCE(" #x ")")
> +#define WARN_ON_ONCE(x) WARN_ONCE((x), "WARN_ON_ONCE(%s)", #x )
>
>  #define MISSING_CASE(x) WARN(1, "Missing switch case (%lu) in %s\n", \
>                              (long) (x), __func__);
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



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

Re: [PATCH] drm/i915: fix driver's versions of WARN_ON & WARN_ON_ONCE

[shuang.he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 7223
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
ILK                 -2              302/302              300/302
SNB                                  315/315              315/315
IVB                                  336/336              336/336
BYT                 -2              283/283              281/283
HSW                                  378/378              378/378
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*ILK  igt@kms_flip@flip-vs-dpms-interruptible      PASS(1)      DMESG_WARN(1)
*ILK  igt@kms_flip@wf_vblank-vs-modeset-interruptible      PASS(1)      DMESG_WARN(1)
*BYT  igt@gem_partial_pwrite_pread@reads-uncached      PASS(1)      FAIL(1)
*BYT  igt@gem_tiled_partial_pwrite_pread@reads      PASS(1)      FAIL(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: fix driver's versions of WARN_ON & WARN_ON_ONCE

[Daniel Vetter]

On Mon, Aug 17, 2015 at 01:45:01PM -0300, Paulo Zanoni wrote:
> 2015-08-17 13:30 GMT-03:00 Dave Gordon <david.s.gordon@intel.com>:
> > The current versions of these two macros don't work correctly if the
> > argument expression happens to contain a modulo operator (%) -- when
> > stringified, it gets interpreted as a printf formatting character!
> > With a specifically crafted parameter, this could probably cause a
> > kernel OOPS; consider WARN_ON(p%s) or WARN_ON(f %*pEp).
> >
> > Instead, we should use an explicit "%s" format, with the stringified
> > expression as the coresponding literal-string argument.
> 
> I suppose that was inspired by the recent IGT commit :)
> 
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx