On 20.02.2020 01:57, Stephen Boyd wrote: > Quoting Leonard Crestez (2020-02-11 13:24:33) >> 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/0x1f0 > > Can 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. >> 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
