On Tue, 13 Apr 2021 19:59:34 +0300 Ville Syrjälä wrote:

On Tue, Apr 13, 2021 at 05:02:40PM +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

---

drivers/gpu/drm/i915/i915_mitigations.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_mitigations.c b/drivers/gpu/drm/i915/i915_mitigations.c index 84f12598d145..7dadf41064e0 100644 --- a/drivers/gpu/drm/i915/i915_mitigations.c +++ b/drivers/gpu/drm/i915/i915_mitigations.c @@ -29,15 +29,13 @@ 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);  
  

I don't think strncpy() guarantees that the string is properly terminated.

Also commit b1b6bed3b503 ("usb: core: fix quirks_param_set() writing to a const pointer") looks broken as well given your findings, and arch/um/drivers/virtio_uml.c seems to suffer from this as well.

wow thank you so much. I will send out patches to fix them as well.

kernel/params.c itself seems to have some slab_is_available() magic around kmalloc().

I used the following cocci snippet to find these:

Nice cocci script.

@find@ identifier O, F; position PS; @@ struct kernel_param_ops O = { ..., .set = F@PS ,... };

@alloc@ identifier ALLOC =~ "^k.*(alloc|dup)"; identifier find.F; position PA; @@ F(...) { <+... ALLOC@PA(...) ...+> }

@script:python depends on alloc@ ps << find.PS; pa << alloc.PA; @@ coccilib.report.print_report(ps[0], "struct") coccilib.report.print_report(pa[0], "alloc")

That could of course miss a bunch more if they allocate via some other function I didn't consider.

-- Ville Syrjälä Intel