i2c: mpc: Avoid out of bounds memory access
Subsystems (2)
Chris Packham
maintainerPATCH 1/1 i2c: mpc: Avoid out of bounds memory access
When performing an I2C transfer where the last message was a write KASAN would complain:
BUG: KASAN: slab-out-of-bounds in mpc_i2c_do_action+0x154/0x630 Read of size 2 at addr c814e310 by task swapper/2/0 CPU: 2 PID: 0 Comm: swapper/2 Tainted: G B 5.16.0-rc8 #1 Call Trace: [e5ee9d50] [c08418e8] dump_stack_lvl+0x4c/0x6c (unreliable) [e5ee9d70] [c02f8a14] print_address_description.constprop.13+0x64/0x3b0 [e5ee9da0] [c02f9030] kasan_report+0x1f0/0x204 [e5ee9de0] [c0c76ee4] mpc_i2c_do_action+0x154/0x630 [e5ee9e30] [c0c782c4] mpc_i2c_isr+0x164/0x240 [e5ee9e60] [c00f3a04] __handle_irq_event_percpu+0xf4/0x3b0 [e5ee9ec0] [c00f3d40] handle_irq_event_percpu+0x80/0x110 [e5ee9f40] [c00f3e48] handle_irq_event+0x78/0xd0 [e5ee9f60] [c00fcfec] handle_fasteoi_irq+0x19c/0x370 [e5ee9fa0] [c00f1d84] generic_handle_irq+0x54/0x80 [e5ee9fc0] [c0006b54] __do_irq+0x64/0x200 [e5ee9ff0] [c0007958] __do_IRQ+0xe8/0x1c0 [c812dd50] [e3eaab20] 0xe3eaab20 [c812dd90] [c0007a4c] do_IRQ+0x1c/0x30 [c812dda0] [c0000c04] ExternalInput+0x144/0x160 --- interrupt: 500 at arch_cpu_idle+0x34/0x60 NIP: c000b684 LR: c000b684 CTR: c0019688 REGS: c812ddb0 TRAP: 0500 Tainted: G B (5.16.0-rc8) MSR: 00029002 <CE,EE,ME> CR: 22000488 XER: 20000000
GPR00: c10ef7fc c812de90 c80ff200 c2394718 00000001 00000001 c10e3f90 00000003
GPR08: 00000000 c0019688 c2394718 fc7d625b 22000484 00000000 21e17000 c208228c
GPR16: e3e99284 00000000 ffffffff c2390000 c001bac0 c2082288 c812df60 c001ba60
GPR24: c23949c0 00000018 00080000 00000004 c80ff200 00000002 c2348ee4 c2394718 NIP [c000b684] arch_cpu_idle+0x34/0x60
LR [c000b684] arch_cpu_idle+0x34/0x60
--- interrupt: 500
[c812de90] [c10e3f90] rcu_eqs_enter.isra.60+0xc0/0x110 (unreliable) [c812deb0] [c10ef7fc] default_idle_call+0xbc/0x230
[c812dee0] [c00af0e8] do_idle+0x1c8/0x200
[c812df10] [c00af3c0] cpu_startup_entry+0x20/0x30
[c812df20] [c001e010] start_secondary+0x5d0/0xba0
[c812dff0] [c00028a0] __secondary_start+0x90/0xdc
This happened because we would overrun the i2c->msgs array on the final interrupt for the I2C STOP. This didn't happen if the last message was a read because there is no interrupt in that case. Ensure that we only access the current message if we are not processing a I2C STOP condition.
--- drivers/i2c/busses/i2c-mpc.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)
drivers/i2c/busses/i2c-mpc.c +9 -6
@@ -492,7 +492,7 @@ static void mpc_i2c_finish(struct mpc_i2c *i2c, int rc)static void mpc_i2c_do_action (struct mpc_i2c *i2c) { -struct i2c_msg *msg = &i2c->msgs[i2c->curr_msg]; +struct i2c_msg *msg =NULL ;int dir =0 ;int recv_len =0 ;u8 byte;
@@ -501,10 +501,13 @@ static void mpc_i2c_do_action(struct mpc_i2c *i2c) i2c->cntl_bits &= ~(CCR_RSTA |CCR_MTX |CCR_TXAK ); -if (msg->flags &I2C_M_RD ) - dir =1 ; -if (msg->flags &I2C_M_RECV_LEN ) - recv_len =1 ; +if (i2c->action !=MPC_I2C_ACTION_STOP ) { + msg = &i2c->msgs[i2c->curr_msg]; +if (msg->flags &I2C_M_RD ) + dir =1 ; +if (msg->flags &I2C_M_RECV_LEN ) + recv_len =1 ; + }switch (i2c->action) {case MPC_I2C_ACTION_RESTART :
@@ -581,7 +584,7 @@ static void mpc_i2c_do_action(struct mpc_i2c *i2c)break ; } -if (msg->len == i2c->byte_posn) { +if (msg && msg->len == i2c->byte_posn) { i2c->curr_msg++; i2c->byte_posn =0 ;
Signature · 1 line
2.34.1
Wolfram Sang
AppliedQuoted 48 lines · Chris Packham
When performing an I2C transfer where the last message was a write KASAN would complain:
BUG: KASAN: slab-out-of-bounds in mpc_i2c_do_action+0x154/0x630 Read of size 2 at addr c814e310 by task swapper/2/0 CPU: 2 PID: 0 Comm: swapper/2 Tainted: G B 5.16.0-rc8 #1 Call Trace: [e5ee9d50] [c08418e8] dump_stack_lvl+0x4c/0x6c (unreliable) [e5ee9d70] [c02f8a14] print_address_description.constprop.13+0x64/0x3b0 [e5ee9da0] [c02f9030] kasan_report+0x1f0/0x204 [e5ee9de0] [c0c76ee4] mpc_i2c_do_action+0x154/0x630 [e5ee9e30] [c0c782c4] mpc_i2c_isr+0x164/0x240 [e5ee9e60] [c00f3a04] __handle_irq_event_percpu+0xf4/0x3b0 [e5ee9ec0] [c00f3d40] handle_irq_event_percpu+0x80/0x110 [e5ee9f40] [c00f3e48] handle_irq_event+0x78/0xd0 [e5ee9f60] [c00fcfec] handle_fasteoi_irq+0x19c/0x370 [e5ee9fa0] [c00f1d84] generic_handle_irq+0x54/0x80 [e5ee9fc0] [c0006b54] __do_irq+0x64/0x200 [e5ee9ff0] [c0007958] __do_IRQ+0xe8/0x1c0 [c812dd50] [e3eaab20] 0xe3eaab20 [c812dd90] [c0007a4c] do_IRQ+0x1c/0x30 [c812dda0] [c0000c04] ExternalInput+0x144/0x160 --- interrupt: 500 at arch_cpu_idle+0x34/0x60 NIP: c000b684 LR: c000b684 CTR: c0019688 REGS: c812ddb0 TRAP: 0500 Tainted: G B (5.16.0-rc8) MSR: 00029002 <CE,EE,ME> CR: 22000488 XER: 20000000GPR00:
c10ef7fcc812de90c80ff200c239471800000001 00000001c10e3f9000000003
GPR08: 00000000c0019688c2394718fc7d625b22000484 0000000021e17000c208228c
GPR16:e3e9928400000000 ffffffffc2390000c001bac0c2082288c812df60c001ba60
GPR24:c23949c000000018 00080000 00000004c80ff20000000002c2348ee4c2394718NIP [c000b684]arch_cpu_idle+0x34/0x60
LR [c000b684]arch_cpu_idle+0x34/0x60
--- interrupt: 500
[c812de90] [c10e3f90] rcu_eqs_enter.isra.60+0xc0/0x110 (unreliable) [c812deb0] [c10ef7fc]default_idle_call+0xbc/0x230
[c812dee0] [c00af0e8]do_idle+0x1c8/0x200
[c812df10] [c00af3c0]cpu_startup_entry+0x20/0x30
[c812df20] [c001e010]start_secondary+0x5d0/0xba0
[c812dff0] [c00028a0]__secondary_start+0x90/0xdcThis happened because we would overrun the
i2c->msgsarray on the final interrupt for the I2C STOP. This didn't happen if the last message was a read because there is no interrupt in that case. Ensure that we only access the current message if we are not processing a I2C STOP condition.
Quoted 3 lines
Fixes 1538d82f4647 ("i2c: mpc: Interrupt driven transfer")Reported-by Maxime Bizon <[email protected]>Signed-off-by Chris Packham <[email protected]>
Applied to for-current, thanks!