Re: [PATCH] firmware: imx: Align imx SC msg structs to 4
From: Leonard Crestez <hidden>
Date: 2020-02-20 12:25:43
Also in:
linux-clk, linux-gpio, linux-rtc
On 20.02.2020 01:57, Stephen Boyd wrote:
Quoting Leonard Crestez (2020-02-11 13:24:33)quoted
The imx SC api strongly assumes that messages are composed out of 4-bytes words but some of our message structs have sizeof "6" and "7". This produces many oopses with CONFIG_KASAN=y: BUG: KASAN: stack-out-of-bounds in imx_mu_send_data+0x108/0x1f0Can you share the full kasan bug report instead of the single line?
[ 1.606708] imx-scu scu: NXP i.MX SCU Initialized [ 1.635265] random: fast init done [ 1.652200] ================================================================== [ 1.659118] BUG: KASAN: stack-out-of-bounds in imx_mu_send_data+0x108/0x1f0 [ 1.666046] Read of size 4 at addr ffff0008c80e6bc4 by task swapper/0/1 [ 1.672642] [ 1.674134] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.4.3-03848-g13efcd6 #54 [ 1.681335] Hardware name: Freescale i.MX8QM MEK (DT) [ 1.686373] Call trace: [ 1.688815] dump_backtrace+0x0/0x1e8 [ 1.692458] show_stack+0x14/0x20 [ 1.695766] dump_stack+0xe8/0x140 [ 1.699155] print_address_description.isra.11+0x64/0x348 [ 1.704532] __kasan_report+0x11c/0x230 [ 1.708356] kasan_report+0xc/0x18 [ 1.711743] __asan_load4+0x90/0xb0 [ 1.715218] imx_mu_send_data+0x108/0x1f0 [ 1.719215] msg_submit+0x104/0x180 [ 1.722689] mbox_send_message+0xa8/0x1a0 [ 1.726696] imx_scu_call_rpc+0x168/0x310 [ 1.730679] imx_sc_pd_power+0x180/0x1e0 [ 1.734589] imx_sc_pd_power_on+0x10/0x18 [ 1.738598] genpd_power_on.part.23+0x118/0x2a8 [ 1.743105] genpd_runtime_resume+0x138/0x320 [ 1.747454] __rpm_callback+0xb0/0x1a0 [ 1.751184] rpm_callback+0x34/0xe0 [ 1.754659] rpm_resume+0x5b8/0x7e8 [ 1.758137] __pm_runtime_resume+0x38/0x90 [ 1.762222] imx_clk_scu_probe+0x5c/0x1c8 [ 1.766218] platform_drv_probe+0x6c/0xc8 [ 1.770217] really_probe+0x148/0x428 [ 1.773861] driver_probe_device+0x74/0x130 [ 1.778031] __device_attach_driver+0xc4/0xe8 [ 1.782380] bus_for_each_drv+0xf0/0x158 [ 1.786283] __device_attach+0x158/0x1d8 [ 1.790195] device_initial_probe+0x10/0x18 [ 1.794362] bus_probe_device+0xe0/0xf0 [ 1.798185] device_add+0x660/0x998 [ 1.801659] platform_device_add+0x198/0x340 [ 1.805916] imx_clk_scu_alloc_dev+0x1b8/0x1e8 [ 1.810347] imx8qxp_clk_probe+0x19d0/0x28b8 [ 1.814601] platform_drv_probe+0x6c/0xc8 [ 1.818601] really_probe+0x148/0x428 [ 1.822250] driver_probe_device+0x74/0x130 [ 1.826423] __device_attach_driver+0xc4/0xe8 [ 1.830763] bus_for_each_drv+0xf0/0x158 [ 1.834675] __device_attach+0x158/0x1d8 [ 1.838583] device_initial_probe+0x10/0x18 [ 1.842752] bus_probe_device+0xe0/0xf0 [ 1.846572] device_add+0x660/0x998 [ 1.850058] of_device_add+0x74/0x98 [ 1.853610] of_platform_device_create_pdata+0x11c/0x178 [ 1.858908] of_platform_bus_create+0x404/0x4f0 [ 1.863425] of_platform_populate+0x74/0x110 [ 1.867688] devm_of_platform_populate+0x54/0xb8 [ 1.872291] imx_scu_probe+0x1b8/0x220 [ 1.876022] platform_drv_probe+0x6c/0xc8 [ 1.880021] really_probe+0x148/0x428 [ 1.883671] driver_probe_device+0x74/0x130 [ 1.887841] device_driver_attach+0x94/0xa0 [ 1.892010] __driver_attach+0x70/0x110 [ 1.895832] bus_for_each_dev+0xe8/0x158 [ 1.899741] driver_attach+0x30/0x40 [ 1.903303] bus_add_driver+0x1b0/0x2b8 [ 1.907129] driver_register+0xbc/0x1d0 [ 1.910947] __platform_driver_register+0x7c/0x88 [ 1.915653] imx_scu_driver_init+0x18/0x20 [ 1.919725] do_one_initcall+0xd4/0x244 [ 1.923552] kernel_init_freeable+0x238/0x2d4 [ 1.927889] kernel_init+0x10/0x114 [ 1.931365] ret_from_fork+0x10/0x18 [ 1.934917] [ 1.936399] The buggy address belongs to the page: [ 1.941184] page:fffffe0023003980 refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 [ 1.949447] raw: 1ffff00000000000 fffffe0023003988 fffffe0023003988 0000000000000000 [ 1.957170] raw: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000 [ 1.964894] page dumped because: kasan: bad access detected [ 1.970449] [ 1.971933] addr ffff0008c80e6bc4 is located in stack of task swapper/0/1 at offset 36 in frame: [ 1.980708] imx_sc_pd_power+0x0/0x1e0 [ 1.984442] [ 1.985917] this frame has 1 object: [ 1.989481] [32, 39) 'msg' [ 1.989484] [ 1.993732] Memory state around the buggy address: [ 1.998520] ffff0008c80e6a80: 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00 [ 2.005730] ffff0008c80e6b00: 00 00 f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 [ 2.012940] >ffff0008c80e6b80: 00 00 00 00 f1 f1 f1 f1 07 f2 f2 f2 00 00 00 00 [ 2.020141] ^ [ 2.025448] ffff0008c80e6c00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 2.032659] ffff0008c80e6c80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ 2.039862] ================================================================== This is actually from an NXP release branch but code is very close up upstream.
quoted
It shouldn't cause an issues in normal use because these structs are always allocated on the stack.Is packed necessary for these? I thought that if the beginning of the struct was naturally aligned and there was sometimes a byte or two at the end then having __packed wasn't useful. So maybe it's better to just drop __packed on all these structs and let the compiler decide it can add some padding on the stack? Or do we have arrays of these structs sitting in memory all next to each other and they need to be that way to be sent through the mailbox?
I'm not sure I understand the question: the structs are __packed because they represent the binary protocol for communicating with the "System Controller". Without __packed the compiler could insert padding inside the structs and break the protocol. As far as I understand compilers are still allowed to use padding on stack since that padding is outside the message struct itself. -- Regards, Leonard _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel