All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
Cc: David Airlie <airlied@linux.ie>,
	intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	Chris Wilson <chris@chris-wilson.co.uk>,
	dri-devel@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v2] drm/i915: Fix "mitigations" parsing if i915 is builtin
Date: Thu, 15 Apr 2021 18:50:37 +0300	[thread overview]
Message-ID: <YHhgzVkSDDkm95FJ@intel.com> (raw)
In-Reply-To: <20210414140643.620c3adb@xhacker.debian>

On Wed, Apr 14, 2021 at 02:06:43PM +0800, Jisheng Zhang wrote:
> I met below error during boot with i915 builtin if pass
> "i915.mitigations=off":
> [    0.015589] Booting kernel: `off' invalid for parameter `i915.mitigations'
> 
> The reason is slab subsystem isn't ready at that time, so kstrdup()
> returns NULL. Fix this issue by using stack var instead of kstrdup().
> 
> Fixes: 984cadea032b ("drm/i915: Allow the sysadmin to override security mitigations")
> Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> ---
> Since v1:
>  - Ensure "str" is properly terminated. Thanks Ville for pointing this out.
> 
>  drivers/gpu/drm/i915/i915_mitigations.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_mitigations.c b/drivers/gpu/drm/i915/i915_mitigations.c
> index 84f12598d145..231aad5ff46c 100644
> --- a/drivers/gpu/drm/i915/i915_mitigations.c
> +++ b/drivers/gpu/drm/i915/i915_mitigations.c
> @@ -29,15 +29,14 @@ bool i915_mitigate_clear_residuals(void)
>  static int mitigations_set(const char *val, const struct kernel_param *kp)
>  {
>  	unsigned long new = ~0UL;
> -	char *str, *sep, *tok;
> +	char str[64], *sep, *tok;
>  	bool first = true;
>  	int err = 0;
>  
>  	BUILD_BUG_ON(ARRAY_SIZE(names) >= BITS_PER_TYPE(mitigations));
>  
> -	str = kstrdup(val, GFP_KERNEL);
> -	if (!str)
> -		return -ENOMEM;
> +	strncpy(str, val, sizeof(str) - 1);
> +	str[sizeof(str) - 1] = '\0';

Looks correct, however strscpy() seems to be the thing we should
be using these days.

>  
>  	for (sep = str; (tok = strsep(&sep, ","));) {
>  		bool enable = true;
> @@ -86,7 +85,6 @@ static int mitigations_set(const char *val, const struct kernel_param *kp)
>  			break;
>  		}
>  	}
> -	kfree(str);
>  	if (err)
>  		return err;
>  
> -- 
> 2.31.0

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

WARNING: multiple messages have this Message-ID (diff)
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
Cc: David Airlie <airlied@linux.ie>,
	intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	Chris Wilson <chris@chris-wilson.co.uk>,
	Jon Bloomfield <jon.bloomfield@intel.com>,
	dri-devel@lists.freedesktop.org,
	Rodrigo Vivi <rodrigo.vivi@intel.com>
Subject: Re: [PATCH v2] drm/i915: Fix "mitigations" parsing if i915 is builtin
Date: Thu, 15 Apr 2021 18:50:37 +0300	[thread overview]
Message-ID: <YHhgzVkSDDkm95FJ@intel.com> (raw)
In-Reply-To: <20210414140643.620c3adb@xhacker.debian>

On Wed, Apr 14, 2021 at 02:06:43PM +0800, Jisheng Zhang wrote:
> I met below error during boot with i915 builtin if pass
> "i915.mitigations=off":
> [    0.015589] Booting kernel: `off' invalid for parameter `i915.mitigations'
> 
> The reason is slab subsystem isn't ready at that time, so kstrdup()
> returns NULL. Fix this issue by using stack var instead of kstrdup().
> 
> Fixes: 984cadea032b ("drm/i915: Allow the sysadmin to override security mitigations")
> Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> ---
> Since v1:
>  - Ensure "str" is properly terminated. Thanks Ville for pointing this out.
> 
>  drivers/gpu/drm/i915/i915_mitigations.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_mitigations.c b/drivers/gpu/drm/i915/i915_mitigations.c
> index 84f12598d145..231aad5ff46c 100644
> --- a/drivers/gpu/drm/i915/i915_mitigations.c
> +++ b/drivers/gpu/drm/i915/i915_mitigations.c
> @@ -29,15 +29,14 @@ bool i915_mitigate_clear_residuals(void)
>  static int mitigations_set(const char *val, const struct kernel_param *kp)
>  {
>  	unsigned long new = ~0UL;
> -	char *str, *sep, *tok;
> +	char str[64], *sep, *tok;
>  	bool first = true;
>  	int err = 0;
>  
>  	BUILD_BUG_ON(ARRAY_SIZE(names) >= BITS_PER_TYPE(mitigations));
>  
> -	str = kstrdup(val, GFP_KERNEL);
> -	if (!str)
> -		return -ENOMEM;
> +	strncpy(str, val, sizeof(str) - 1);
> +	str[sizeof(str) - 1] = '\0';

Looks correct, however strscpy() seems to be the thing we should
be using these days.

>  
>  	for (sep = str; (tok = strsep(&sep, ","));) {
>  		bool enable = true;
> @@ -86,7 +85,6 @@ static int mitigations_set(const char *val, const struct kernel_param *kp)
>  			break;
>  		}
>  	}
> -	kfree(str);
>  	if (err)
>  		return err;
>  
> -- 
> 2.31.0

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

WARNING: multiple messages have this Message-ID (diff)
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>,
	Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>,
	David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
	Jon Bloomfield <jon.bloomfield@intel.com>,
	Chris Wilson <chris@chris-wilson.co.uk>,
	intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] drm/i915: Fix "mitigations" parsing if i915 is builtin
Date: Thu, 15 Apr 2021 18:50:37 +0300	[thread overview]
Message-ID: <YHhgzVkSDDkm95FJ@intel.com> (raw)
In-Reply-To: <20210414140643.620c3adb@xhacker.debian>

On Wed, Apr 14, 2021 at 02:06:43PM +0800, Jisheng Zhang wrote:
> I met below error during boot with i915 builtin if pass
> "i915.mitigations=off":
> [    0.015589] Booting kernel: `off' invalid for parameter `i915.mitigations'
> 
> The reason is slab subsystem isn't ready at that time, so kstrdup()
> returns NULL. Fix this issue by using stack var instead of kstrdup().
> 
> Fixes: 984cadea032b ("drm/i915: Allow the sysadmin to override security mitigations")
> Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> ---
> Since v1:
>  - Ensure "str" is properly terminated. Thanks Ville for pointing this out.
> 
>  drivers/gpu/drm/i915/i915_mitigations.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_mitigations.c b/drivers/gpu/drm/i915/i915_mitigations.c
> index 84f12598d145..231aad5ff46c 100644
> --- a/drivers/gpu/drm/i915/i915_mitigations.c
> +++ b/drivers/gpu/drm/i915/i915_mitigations.c
> @@ -29,15 +29,14 @@ bool i915_mitigate_clear_residuals(void)
>  static int mitigations_set(const char *val, const struct kernel_param *kp)
>  {
>  	unsigned long new = ~0UL;
> -	char *str, *sep, *tok;
> +	char str[64], *sep, *tok;
>  	bool first = true;
>  	int err = 0;
>  
>  	BUILD_BUG_ON(ARRAY_SIZE(names) >= BITS_PER_TYPE(mitigations));
>  
> -	str = kstrdup(val, GFP_KERNEL);
> -	if (!str)
> -		return -ENOMEM;
> +	strncpy(str, val, sizeof(str) - 1);
> +	str[sizeof(str) - 1] = '\0';

Looks correct, however strscpy() seems to be the thing we should
be using these days.

>  
>  	for (sep = str; (tok = strsep(&sep, ","));) {
>  		bool enable = true;
> @@ -86,7 +85,6 @@ static int mitigations_set(const char *val, const struct kernel_param *kp)
>  			break;
>  		}
>  	}
> -	kfree(str);
>  	if (err)
>  		return err;
>  
> -- 
> 2.31.0

-- 
Ville Syrjälä
Intel

  parent reply	other threads:[~2021-04-15 15:50 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-14  6:06 [Intel-gfx] [PATCH v2] drm/i915: Fix "mitigations" parsing if i915 is builtin Jisheng Zhang
2021-04-14  6:06 ` Jisheng Zhang
2021-04-14  6:06 ` Jisheng Zhang
2021-04-14 12:38 ` [Intel-gfx] ✗ Fi.CI.DOCS: warning for drm/i915: Fix "mitigations" parsing if i915 is builtin (rev2) Patchwork
2021-04-14 13:03 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-04-14 16:47 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2021-04-15 15:50 ` Ville Syrjälä [this message]
2021-04-15 15:50   ` [PATCH v2] drm/i915: Fix "mitigations" parsing if i915 is builtin Ville Syrjälä
2021-04-15 15:50   ` Ville Syrjälä

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YHhgzVkSDDkm95FJ@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=Jisheng.Zhang@synaptics.com \
    --cc=airlied@linux.ie \
    --cc=chris@chris-wilson.co.uk \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
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.