| Message ID | 20241023203032.1388491-1-Frank.Li@nxp.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 25f00a13dccf8e45441265768de46c8bf58e08f6 |
| Headers | show |
| Series | [1/1] spi: spi-fsl-dspi: Fix crash when not using GPIO chip select | expand |
Hi Frank, On Wed, Oct 23, 2024 at 04:30:32PM -0400, Frank Li wrote: > Add check for the return value of spi_get_csgpiod() to avoid passing a NULL > pointer to gpiod_direction_output(), preventing a crash when GPIO chip > select is not used. > > Fix below crash: > [ 4.251960] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000 > [ 4.260762] Mem abort info: > [ 4.263556] ESR = 0x0000000096000004 > [ 4.267308] EC = 0x25: DABT (current EL), IL = 32 bits > [ 4.272624] SET = 0, FnV = 0 > [ 4.275681] EA = 0, S1PTW = 0 > [ 4.278822] FSC = 0x04: level 0 translation fault > [ 4.283704] Data abort info: > [ 4.286583] ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000 > [ 4.292074] CM = 0, WnR = 0, TnD = 0, TagAccess = 0 > [ 4.297130] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 > [ 4.302445] [0000000000000000] user address but active_mm is swapper > [ 4.308805] Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP > [ 4.315072] Modules linked in: > [ 4.318124] CPU: 2 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.12.0-rc4-next-20241023-00008-ga20ec42c5fc1 #359 > [ 4.328130] Hardware name: LS1046A QDS Board (DT) > [ 4.332832] pstate: 40000005 (nZcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) > [ 4.339794] pc : gpiod_direction_output+0x34/0x5c > [ 4.344505] lr : gpiod_direction_output+0x18/0x5c > [ 4.349208] sp : ffff80008003b8f0 > [ 4.352517] x29: ffff80008003b8f0 x28: 0000000000000000 x27: ffffc96bcc7e9068 > [ 4.359659] x26: ffffc96bcc6e00b0 x25: ffffc96bcc598398 x24: ffff447400132810 > [ 4.366800] x23: 0000000000000000 x22: 0000000011e1a300 x21: 0000000000020002 > [ 4.373940] x20: 0000000000000000 x19: 0000000000000000 x18: ffffffffffffffff > [ 4.381081] x17: ffff44740016e600 x16: 0000000500000003 x15: 0000000000000007 > [ 4.388221] x14: 0000000000989680 x13: 0000000000020000 x12: 000000000000001e > [ 4.395362] x11: 0044b82fa09b5a53 x10: 0000000000000019 x9 : 0000000000000008 > [ 4.402502] x8 : 0000000000000002 x7 : 0000000000000007 x6 : 0000000000000000 > [ 4.409641] x5 : 0000000000000200 x4 : 0000000002000000 x3 : 0000000000000000 > [ 4.416781] x2 : 0000000000022202 x1 : 0000000000000000 x0 : 0000000000000000 > [ 4.423921] Call trace: > [ 4.426362] gpiod_direction_output+0x34/0x5c (P) > [ 4.431067] gpiod_direction_output+0x18/0x5c (L) > [ 4.435771] dspi_setup+0x220/0x334 > > Fixes: 9e264f3f85a5 ("spi: Replace all spi->chip_select and spi->cs_gpiod references with function call") > Cc: stable@vger.kernel.org > Signed-off-by: Frank Li <Frank.Li@nxp.com> > --- Sadly I do not reproduce this NULL pointer dereference. I also know from hearsay that the GPIO consumer API is more or less NULL-tolerant. Could you "make drivers/gpio/gpiolib.lst" and see what's at offsets 0x18 and 0x34 into gpiod_direction_output()? The first thing in that function is the VALIDATE_DESC() macro, which does: static int validate_desc(const struct gpio_desc *desc, const char *func) { if (!desc) return 0; if (IS_ERR(desc)) { pr_warn("%s: invalid GPIO (errorpointer)\n", func); return PTR_ERR(desc); } return 1; } #define VALIDATE_DESC(desc) do { \ int __valid = validate_desc(desc, __func__); \ if (__valid <= 0) \ return __valid; \ } while (0)
On Wed, 23 Oct 2024 16:30:32 -0400, Frank Li wrote: > Add check for the return value of spi_get_csgpiod() to avoid passing a NULL > pointer to gpiod_direction_output(), preventing a crash when GPIO chip > select is not used. > > Fix below crash: > [ 4.251960] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000 > [ 4.260762] Mem abort info: > [ 4.263556] ESR = 0x0000000096000004 > [ 4.267308] EC = 0x25: DABT (current EL), IL = 32 bits > [ 4.272624] SET = 0, FnV = 0 > [ 4.275681] EA = 0, S1PTW = 0 > [ 4.278822] FSC = 0x04: level 0 translation fault > [ 4.283704] Data abort info: > [ 4.286583] ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000 > [ 4.292074] CM = 0, WnR = 0, TnD = 0, TagAccess = 0 > [ 4.297130] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 > [ 4.302445] [0000000000000000] user address but active_mm is swapper > [ 4.308805] Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP > [ 4.315072] Modules linked in: > [ 4.318124] CPU: 2 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.12.0-rc4-next-20241023-00008-ga20ec42c5fc1 #359 > [ 4.328130] Hardware name: LS1046A QDS Board (DT) > [ 4.332832] pstate: 40000005 (nZcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) > [ 4.339794] pc : gpiod_direction_output+0x34/0x5c > [ 4.344505] lr : gpiod_direction_output+0x18/0x5c > [ 4.349208] sp : ffff80008003b8f0 > [ 4.352517] x29: ffff80008003b8f0 x28: 0000000000000000 x27: ffffc96bcc7e9068 > [ 4.359659] x26: ffffc96bcc6e00b0 x25: ffffc96bcc598398 x24: ffff447400132810 > [ 4.366800] x23: 0000000000000000 x22: 0000000011e1a300 x21: 0000000000020002 > [ 4.373940] x20: 0000000000000000 x19: 0000000000000000 x18: ffffffffffffffff > [ 4.381081] x17: ffff44740016e600 x16: 0000000500000003 x15: 0000000000000007 > [ 4.388221] x14: 0000000000989680 x13: 0000000000020000 x12: 000000000000001e > [ 4.395362] x11: 0044b82fa09b5a53 x10: 0000000000000019 x9 : 0000000000000008 > [ 4.402502] x8 : 0000000000000002 x7 : 0000000000000007 x6 : 0000000000000000 > [ 4.409641] x5 : 0000000000000200 x4 : 0000000002000000 x3 : 0000000000000000 > [ 4.416781] x2 : 0000000000022202 x1 : 0000000000000000 x0 : 0000000000000000 > [ 4.423921] Call trace: > [ 4.426362] gpiod_direction_output+0x34/0x5c (P) > [ 4.431067] gpiod_direction_output+0x18/0x5c (L) > [ 4.435771] dspi_setup+0x220/0x334 > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next Thanks! [1/1] spi: spi-fsl-dspi: Fix crash when not using GPIO chip select commit: 25f00a13dccf8e45441265768de46c8bf58e08f6 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c index b52950a0f5f92..067c954cb6ea0 100644 --- a/drivers/spi/spi-fsl-dspi.c +++ b/drivers/spi/spi-fsl-dspi.c @@ -1003,6 +1003,7 @@ static int dspi_setup(struct spi_device *spi) u32 cs_sck_delay = 0, sck_cs_delay = 0; struct fsl_dspi_platform_data *pdata; unsigned char pasc = 0, asc = 0; + struct gpio_desc *gpio_cs; struct chip_data *chip; unsigned long clkrate; bool cs = true; @@ -1077,7 +1078,10 @@ static int dspi_setup(struct spi_device *spi) chip->ctar_val |= SPI_CTAR_LSBFE; } - gpiod_direction_output(spi_get_csgpiod(spi, 0), false); + gpio_cs = spi_get_csgpiod(spi, 0); + if (gpio_cs) + gpiod_direction_output(gpio_cs, false); + dspi_deassert_cs(spi, &cs); spi_set_ctldata(spi, chip);
Add check for the return value of spi_get_csgpiod() to avoid passing a NULL pointer to gpiod_direction_output(), preventing a crash when GPIO chip select is not used. Fix below crash: [ 4.251960] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000 [ 4.260762] Mem abort info: [ 4.263556] ESR = 0x0000000096000004 [ 4.267308] EC = 0x25: DABT (current EL), IL = 32 bits [ 4.272624] SET = 0, FnV = 0 [ 4.275681] EA = 0, S1PTW = 0 [ 4.278822] FSC = 0x04: level 0 translation fault [ 4.283704] Data abort info: [ 4.286583] ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000 [ 4.292074] CM = 0, WnR = 0, TnD = 0, TagAccess = 0 [ 4.297130] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 [ 4.302445] [0000000000000000] user address but active_mm is swapper [ 4.308805] Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP [ 4.315072] Modules linked in: [ 4.318124] CPU: 2 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.12.0-rc4-next-20241023-00008-ga20ec42c5fc1 #359 [ 4.328130] Hardware name: LS1046A QDS Board (DT) [ 4.332832] pstate: 40000005 (nZcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 4.339794] pc : gpiod_direction_output+0x34/0x5c [ 4.344505] lr : gpiod_direction_output+0x18/0x5c [ 4.349208] sp : ffff80008003b8f0 [ 4.352517] x29: ffff80008003b8f0 x28: 0000000000000000 x27: ffffc96bcc7e9068 [ 4.359659] x26: ffffc96bcc6e00b0 x25: ffffc96bcc598398 x24: ffff447400132810 [ 4.366800] x23: 0000000000000000 x22: 0000000011e1a300 x21: 0000000000020002 [ 4.373940] x20: 0000000000000000 x19: 0000000000000000 x18: ffffffffffffffff [ 4.381081] x17: ffff44740016e600 x16: 0000000500000003 x15: 0000000000000007 [ 4.388221] x14: 0000000000989680 x13: 0000000000020000 x12: 000000000000001e [ 4.395362] x11: 0044b82fa09b5a53 x10: 0000000000000019 x9 : 0000000000000008 [ 4.402502] x8 : 0000000000000002 x7 : 0000000000000007 x6 : 0000000000000000 [ 4.409641] x5 : 0000000000000200 x4 : 0000000002000000 x3 : 0000000000000000 [ 4.416781] x2 : 0000000000022202 x1 : 0000000000000000 x0 : 0000000000000000 [ 4.423921] Call trace: [ 4.426362] gpiod_direction_output+0x34/0x5c (P) [ 4.431067] gpiod_direction_output+0x18/0x5c (L) [ 4.435771] dspi_setup+0x220/0x334 Fixes: 9e264f3f85a5 ("spi: Replace all spi->chip_select and spi->cs_gpiod references with function call") Cc: stable@vger.kernel.org Signed-off-by: Frank Li <Frank.Li@nxp.com> --- drivers/spi/spi-fsl-dspi.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)