Messages in this thread | | | Date | Tue, 13 Apr 2021 19:59:34 +0300 | From | Ville Syrjälä <> | Subject | Re: [PATCH] drm/i915: Fix "mitigations" parsing if i915 is builtin |
| |
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. kernel/params.c itself seems to have some slab_is_available() magic around kmalloc().
I used the following cocci snippet to find these: @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
| |