| Message ID | 20240507064434.3213933-1-m.szyprowski@samsung.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream, archived |
| Headers | show |
| Series | clkdev: fix potential NULL pointer dereference | expand |
On Tue, May 07, 2024 at 08:44:34AM +0200, Marek Szyprowski wrote: > dev_fmt argument is optional, so avoid dereferencing it unconditionally. > > Fixes: 4d11c62ca8d7 ("clkdev: report over-sized strings when creating clkdev entries") > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> Please put this in the patch system so it can be merged along with the change that created the problem. Thanks.
Hi Marek, On Tue, 2024-05-07 at 08:44 +0200, Marek Szyprowski wrote: > diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c > index ddacab7863d0..d2801ae70e34 100644 > --- a/drivers/clk/clkdev.c > +++ b/drivers/clk/clkdev.c > @@ -194,10 +194,12 @@ vclkdev_alloc(struct clk_hw *hw, const char *con_id, const char *dev_fmt, > return &cla->cl; > > fail: > - fmt.fmt = dev_fmt; > - fmt.va = &ap_copy; > - pr_err("%pV:%s: %s ID is greater than %zu\n", > - &fmt, con_id, failure, max_size); > + if (dev_fmt) { > + fmt.fmt = dev_fmt; > + fmt.va = &ap_copy; > + pr_err("%pV:%s: %s ID is greater than %zu\n", > + &fmt, con_id, failure, max_size); > + } It might be nice to still print the rest of the error, so it's easier to see which clock is causing trouble. Cheers, Andre'
On Tue, May 07, 2024 at 05:42:45PM +0100, André Draszik wrote: > Hi Marek, > > On Tue, 2024-05-07 at 08:44 +0200, Marek Szyprowski wrote: > > diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c > > index ddacab7863d0..d2801ae70e34 100644 > > --- a/drivers/clk/clkdev.c > > +++ b/drivers/clk/clkdev.c > > @@ -194,10 +194,12 @@ vclkdev_alloc(struct clk_hw *hw, const char *con_id, const char *dev_fmt, > > return &cla->cl; > > > > fail: > > - fmt.fmt = dev_fmt; > > - fmt.va = &ap_copy; > > - pr_err("%pV:%s: %s ID is greater than %zu\n", > > - &fmt, con_id, failure, max_size); > > + if (dev_fmt) { > > + fmt.fmt = dev_fmt; > > + fmt.va = &ap_copy; > > + pr_err("%pV:%s: %s ID is greater than %zu\n", > > + &fmt, con_id, failure, max_size); > > + } > > It might be nice to still print the rest of the error, so it's easier to see which > clock is causing trouble. Good point. I'll fix the patch myself, merging the fix in.
On Tue, May 7, 2024 at 1:45 AM Marek Szyprowski <m.szyprowski@samsung.com> wrote: > > dev_fmt argument is optional, so avoid dereferencing it unconditionally. > > Fixes: 4d11c62ca8d7 ("clkdev: report over-sized strings when creating clkdev entries") > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > Thanks for fixing this, Marek! I tested it on the E850-96 board, and it fixes the boot on current kernel-next for me (next-20240508). Feel free to add: Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org> Tested-by: Sam Protsenko <semen.protsenko@linaro.org> [snip]
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c index ddacab7863d0..d2801ae70e34 100644 --- a/drivers/clk/clkdev.c +++ b/drivers/clk/clkdev.c @@ -194,10 +194,12 @@ vclkdev_alloc(struct clk_hw *hw, const char *con_id, const char *dev_fmt, return &cla->cl; fail: - fmt.fmt = dev_fmt; - fmt.va = &ap_copy; - pr_err("%pV:%s: %s ID is greater than %zu\n", - &fmt, con_id, failure, max_size); + if (dev_fmt) { + fmt.fmt = dev_fmt; + fmt.va = &ap_copy; + pr_err("%pV:%s: %s ID is greater than %zu\n", + &fmt, con_id, failure, max_size); + } va_end(ap_copy); kfree(cla); return NULL;
dev_fmt argument is optional, so avoid dereferencing it unconditionally. Fixes: 4d11c62ca8d7 ("clkdev: report over-sized strings when creating clkdev entries") Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> --- This fixes the following kernel panic observed on Samsung Exynos542x SoCs family: 8<--- cut here --- Unable to handle kernel NULL pointer dereference at virtual address 00000000 when read [00000000] *pgd=00000000 Internal error: Oops: 5 [#1] PREEMPT SMP ARM Modules linked in: CPU: 0 PID: 0 Comm: swapper/0 Not tainted 6.8.0-00001-g4d11c62ca8d7 #14975 Hardware name: Samsung Exynos (Flattened Device Tree) PC is at vsnprintf+0x4c/0x3c8 LR is at init_stack+0x1dd6/0x2000 pc : [<c0c09d3c>] lr : [<c1301dd6>] psr: 800000d3 ... Flags: Nzcv IRQs off FIQs off Mode SVC_32 ISA ARM Segment none Control: 10c5387d Table: 4000406a DAC: 00000051 ... Process swapper/0 (pid: 0, stack limit = 0x(ptrval)) Stack: (0xc1301c70 to 0xc1302000) ... vsnprintf from va_format.constprop.0+0x70/0x160 va_format.constprop.0 from pointer+0x454/0x670 pointer from vsnprintf+0x228/0x3c8 vsnprintf from vprintk_store+0x100/0x428 vprintk_store from vprintk_emit+0x6c/0x328 vprintk_emit from vprintk_default+0x24/0x2c vprintk_default from _printk+0x28/0x4c _printk from vclkdev_alloc+0xf0/0x110 vclkdev_alloc from clkdev_hw_create+0x28/0x88 clkdev_hw_create from clk_hw_register_clkdev+0x38/0x3c clk_hw_register_clkdev from samsung_clk_register_fixed_rate+0xa4/0xd4 samsung_clk_register_fixed_rate from exynos5x_clk_init+0xf0/0x2b0 exynos5x_clk_init from of_clk_init+0x15c/0x228 of_clk_init from time_init+0x24/0x30 time_init from start_kernel+0x4b8/0x620 start_kernel from 0x0 Code: e09e9001 e1a05002 e28da01c 2a000086 (e5d50000) ---[ end trace 0000000000000000 ]--- Kernel panic - not syncing: Attempted to kill the idle task! ---[ end Kernel panic - not syncing: Attempted to kill the idle task! ]--- Best regards Marek Szyprowski, PhD Samsung R&D Institute Poland --- drivers/clk/clkdev.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)