* [PATCH] nvmem: Fix shift-out-of-bound (UBSAN) with byte size cells
@ 2021-10-13 12:45 Srinivas Kandagatla
0 siblings, 0 replies; 3+ messages in thread
From: Srinivas Kandagatla @ 2021-10-13 12:45 UTC (permalink / raw)
To: gregkh
Cc: linux-kernel, Stephen Boyd, Douglas Anderson, stable,
Srinivas Kandagatla
From: Stephen Boyd <swboyd@chromium.org>
If a cell has 'nbits' equal to a multiple of BITS_PER_BYTE the logic
*p &= GENMASK((cell->nbits%BITS_PER_BYTE) - 1, 0);
will become undefined behavior because nbits modulo BITS_PER_BYTE is 0, and we
subtract one from that making a large number that is then shifted more than the
number of bits that fit into an unsigned long.
UBSAN reports this problem:
UBSAN: shift-out-of-bounds in drivers/nvmem/core.c:1386:8
shift exponent 64 is too large for 64-bit type 'unsigned long'
CPU: 6 PID: 7 Comm: kworker/u16:0 Not tainted 5.15.0-rc3+ #9
Hardware name: Google Lazor (rev3+) with KB Backlight (DT)
Workqueue: events_unbound deferred_probe_work_func
Call trace:
dump_backtrace+0x0/0x170
show_stack+0x24/0x30
dump_stack_lvl+0x64/0x7c
dump_stack+0x18/0x38
ubsan_epilogue+0x10/0x54
__ubsan_handle_shift_out_of_bounds+0x180/0x194
__nvmem_cell_read+0x1ec/0x21c
nvmem_cell_read+0x58/0x94
nvmem_cell_read_variable_common+0x4c/0xb0
nvmem_cell_read_variable_le_u32+0x40/0x100
a6xx_gpu_init+0x170/0x2f4
adreno_bind+0x174/0x284
component_bind_all+0xf0/0x264
msm_drm_bind+0x1d8/0x7a0
try_to_bring_up_master+0x164/0x1ac
__component_add+0xbc/0x13c
component_add+0x20/0x2c
dp_display_probe+0x340/0x384
platform_probe+0xc0/0x100
really_probe+0x110/0x304
__driver_probe_device+0xb8/0x120
driver_probe_device+0x4c/0xfc
__device_attach_driver+0xb0/0x128
bus_for_each_drv+0x90/0xdc
__device_attach+0xc8/0x174
device_initial_probe+0x20/0x2c
bus_probe_device+0x40/0xa4
deferred_probe_work_func+0x7c/0xb8
process_one_work+0x128/0x21c
process_scheduled_works+0x40/0x54
worker_thread+0x1ec/0x2a8
kthread+0x138/0x158
ret_from_fork+0x10/0x20
Fix it by making sure there are any bits to mask out.
Cc: Douglas Anderson <dianders@chromium.org>
Fixes: 69aba7948cbe ("nvmem: Add a simple NVMEM framework for consumers")
Cc: stable@vger.kernel.org
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
---
Hi Greg,
Could you pick this up for next possible 5.15 rc.
Thanks,
--srini
drivers/nvmem/core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 3d87fadaa160..8976da38b375 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -1383,7 +1383,8 @@ static void nvmem_shift_read_buffer_in_place(struct nvmem_cell *cell, void *buf)
*p-- = 0;
/* clear msb bits if any leftover in the last byte */
- *p &= GENMASK((cell->nbits%BITS_PER_BYTE) - 1, 0);
+ if (cell->nbits % BITS_PER_BYTE)
+ *p &= GENMASK((cell->nbits % BITS_PER_BYTE) - 1, 0);
}
static int __nvmem_cell_read(struct nvmem_device *nvmem,
--
2.21.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] nvmem: Fix shift-out-of-bound (UBSAN) with byte size cells
2021-09-28 0:06 Stephen Boyd
@ 2021-09-28 10:22 ` Srinivas Kandagatla
0 siblings, 0 replies; 3+ messages in thread
From: Srinivas Kandagatla @ 2021-09-28 10:22 UTC (permalink / raw)
To: Stephen Boyd; +Cc: linux-kernel, Douglas Anderson
On 28/09/2021 01:06, Stephen Boyd wrote:
> If a cell has 'nbits' equal to a multiple of BITS_PER_BYTE the logic
>
> *p &= GENMASK((cell->nbits%BITS_PER_BYTE) - 1, 0);
>
> will become undefined behavior because nbits modulo BITS_PER_BYTE is 0, and we
> subtract one from that making a large number that is then shifted more than the
> number of bits that fit into an unsigned long.
>
> UBSAN reports this problem:
>
> UBSAN: shift-out-of-bounds in drivers/nvmem/core.c:1386:8
> shift exponent 64 is too large for 64-bit type 'unsigned long'
> CPU: 6 PID: 7 Comm: kworker/u16:0 Not tainted 5.15.0-rc3+ #9
> Hardware name: Google Lazor (rev3+) with KB Backlight (DT)
> Workqueue: events_unbound deferred_probe_work_func
> Call trace:
> dump_backtrace+0x0/0x170
> show_stack+0x24/0x30
> dump_stack_lvl+0x64/0x7c
> dump_stack+0x18/0x38
> ubsan_epilogue+0x10/0x54
> __ubsan_handle_shift_out_of_bounds+0x180/0x194
> __nvmem_cell_read+0x1ec/0x21c
> nvmem_cell_read+0x58/0x94
> nvmem_cell_read_variable_common+0x4c/0xb0
> nvmem_cell_read_variable_le_u32+0x40/0x100
> a6xx_gpu_init+0x170/0x2f4
> adreno_bind+0x174/0x284
> component_bind_all+0xf0/0x264
> msm_drm_bind+0x1d8/0x7a0
> try_to_bring_up_master+0x164/0x1ac
> __component_add+0xbc/0x13c
> component_add+0x20/0x2c
> dp_display_probe+0x340/0x384
> platform_probe+0xc0/0x100
> really_probe+0x110/0x304
> __driver_probe_device+0xb8/0x120
> driver_probe_device+0x4c/0xfc
> __device_attach_driver+0xb0/0x128
> bus_for_each_drv+0x90/0xdc
> __device_attach+0xc8/0x174
> device_initial_probe+0x20/0x2c
> bus_probe_device+0x40/0xa4
> deferred_probe_work_func+0x7c/0xb8
> process_one_work+0x128/0x21c
> process_scheduled_works+0x40/0x54
> worker_thread+0x1ec/0x2a8
> kthread+0x138/0x158
> ret_from_fork+0x10/0x20
>
> Fix it by making sure there are any bits to mask out.
>
> Cc: Douglas Anderson <dianders@chromium.org>
> Fixes: 69aba7948cbe ("nvmem: Add a simple NVMEM framework for consumers")
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
Applied thanks,
--srini
> ---
> drivers/nvmem/core.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index 3d87fadaa160..8976da38b375 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -1383,7 +1383,8 @@ static void nvmem_shift_read_buffer_in_place(struct nvmem_cell *cell, void *buf)
> *p-- = 0;
>
> /* clear msb bits if any leftover in the last byte */
> - *p &= GENMASK((cell->nbits%BITS_PER_BYTE) - 1, 0);
> + if (cell->nbits % BITS_PER_BYTE)
> + *p &= GENMASK((cell->nbits % BITS_PER_BYTE) - 1, 0);
> }
>
> static int __nvmem_cell_read(struct nvmem_device *nvmem,
>
> base-commit: e4e737bb5c170df6135a127739a9e6148ee3da82
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] nvmem: Fix shift-out-of-bound (UBSAN) with byte size cells
@ 2021-09-28 0:06 Stephen Boyd
2021-09-28 10:22 ` Srinivas Kandagatla
0 siblings, 1 reply; 3+ messages in thread
From: Stephen Boyd @ 2021-09-28 0:06 UTC (permalink / raw)
To: Srinivas Kandagatla; +Cc: linux-kernel, Douglas Anderson
If a cell has 'nbits' equal to a multiple of BITS_PER_BYTE the logic
*p &= GENMASK((cell->nbits%BITS_PER_BYTE) - 1, 0);
will become undefined behavior because nbits modulo BITS_PER_BYTE is 0, and we
subtract one from that making a large number that is then shifted more than the
number of bits that fit into an unsigned long.
UBSAN reports this problem:
UBSAN: shift-out-of-bounds in drivers/nvmem/core.c:1386:8
shift exponent 64 is too large for 64-bit type 'unsigned long'
CPU: 6 PID: 7 Comm: kworker/u16:0 Not tainted 5.15.0-rc3+ #9
Hardware name: Google Lazor (rev3+) with KB Backlight (DT)
Workqueue: events_unbound deferred_probe_work_func
Call trace:
dump_backtrace+0x0/0x170
show_stack+0x24/0x30
dump_stack_lvl+0x64/0x7c
dump_stack+0x18/0x38
ubsan_epilogue+0x10/0x54
__ubsan_handle_shift_out_of_bounds+0x180/0x194
__nvmem_cell_read+0x1ec/0x21c
nvmem_cell_read+0x58/0x94
nvmem_cell_read_variable_common+0x4c/0xb0
nvmem_cell_read_variable_le_u32+0x40/0x100
a6xx_gpu_init+0x170/0x2f4
adreno_bind+0x174/0x284
component_bind_all+0xf0/0x264
msm_drm_bind+0x1d8/0x7a0
try_to_bring_up_master+0x164/0x1ac
__component_add+0xbc/0x13c
component_add+0x20/0x2c
dp_display_probe+0x340/0x384
platform_probe+0xc0/0x100
really_probe+0x110/0x304
__driver_probe_device+0xb8/0x120
driver_probe_device+0x4c/0xfc
__device_attach_driver+0xb0/0x128
bus_for_each_drv+0x90/0xdc
__device_attach+0xc8/0x174
device_initial_probe+0x20/0x2c
bus_probe_device+0x40/0xa4
deferred_probe_work_func+0x7c/0xb8
process_one_work+0x128/0x21c
process_scheduled_works+0x40/0x54
worker_thread+0x1ec/0x2a8
kthread+0x138/0x158
ret_from_fork+0x10/0x20
Fix it by making sure there are any bits to mask out.
Cc: Douglas Anderson <dianders@chromium.org>
Fixes: 69aba7948cbe ("nvmem: Add a simple NVMEM framework for consumers")
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
drivers/nvmem/core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index 3d87fadaa160..8976da38b375 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -1383,7 +1383,8 @@ static void nvmem_shift_read_buffer_in_place(struct nvmem_cell *cell, void *buf)
*p-- = 0;
/* clear msb bits if any leftover in the last byte */
- *p &= GENMASK((cell->nbits%BITS_PER_BYTE) - 1, 0);
+ if (cell->nbits % BITS_PER_BYTE)
+ *p &= GENMASK((cell->nbits % BITS_PER_BYTE) - 1, 0);
}
static int __nvmem_cell_read(struct nvmem_device *nvmem,
base-commit: e4e737bb5c170df6135a127739a9e6148ee3da82
--
https://chromeos.dev
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-10-13 12:45 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-13 12:45 [PATCH] nvmem: Fix shift-out-of-bound (UBSAN) with byte size cells Srinivas Kandagatla
-- strict thread matches above, loose matches on Subject: below --
2021-09-28 0:06 Stephen Boyd
2021-09-28 10:22 ` Srinivas Kandagatla
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).