Skip to content

Commit 54ec8b0

Browse files
pchelkin91marckleinebudde
authored andcommitted
can: kvaser_pciefd: refine error prone echo_skb_max handling logic
echo_skb_max should define the supported upper limit of echo_skb[] allocated inside the netdevice's priv. The corresponding size value provided by this driver to alloc_candev() is KVASER_PCIEFD_CAN_TX_MAX_COUNT which is 17. But later echo_skb_max is rounded up to the nearest power of two (for the max case, that would be 32) and the tx/ack indices calculated further during tx/rx may exceed the upper array boundary. Kasan reported this for the ack case inside kvaser_pciefd_handle_ack_packet(), though the xmit function has actually caught the same thing earlier. BUG: KASAN: slab-out-of-bounds in kvaser_pciefd_handle_ack_packet+0x2d7/0x92a drivers/net/can/kvaser_pciefd.c:1528 Read of size 8 at addr ffff888105e4f078 by task swapper/4/0 CPU: 4 UID: 0 PID: 0 Comm: swapper/4 Not tainted 6.15.0 #12 PREEMPT(voluntary) Call Trace: <IRQ> dump_stack_lvl lib/dump_stack.c:122 print_report mm/kasan/report.c:521 kasan_report mm/kasan/report.c:634 kvaser_pciefd_handle_ack_packet drivers/net/can/kvaser_pciefd.c:1528 kvaser_pciefd_read_packet drivers/net/can/kvaser_pciefd.c:1605 kvaser_pciefd_read_buffer drivers/net/can/kvaser_pciefd.c:1656 kvaser_pciefd_receive_irq drivers/net/can/kvaser_pciefd.c:1684 kvaser_pciefd_irq_handler drivers/net/can/kvaser_pciefd.c:1733 __handle_irq_event_percpu kernel/irq/handle.c:158 handle_irq_event kernel/irq/handle.c:210 handle_edge_irq kernel/irq/chip.c:833 __common_interrupt arch/x86/kernel/irq.c:296 common_interrupt arch/x86/kernel/irq.c:286 </IRQ> Tx max count definitely matters for kvaser_pciefd_tx_avail(), but for seq numbers' generation that's not the case - we're free to calculate them as would be more convenient, not taking tx max count into account. The only downside is that the size of echo_skb[] should correspond to the max seq number (not tx max count), so in some situations a bit more memory would be consumed than could be. Thus make the size of the underlying echo_skb[] sufficient for the rounded max tx value. Found by Linux Verification Center (linuxtesting.org) with Syzkaller. Fixes: 8256e0c ("can: kvaser_pciefd: Fix echo_skb race") Cc: [email protected] Signed-off-by: Fedor Pchelkin <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Marc Kleine-Budde <[email protected]>
1 parent 271683b commit 54ec8b0

File tree

1 file changed

+1
-2
lines changed

1 file changed

+1
-2
lines changed

drivers/net/can/kvaser_pciefd.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -966,7 +966,7 @@ static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie)
966966
u32 status, tx_nr_packets_max;
967967

968968
netdev = alloc_candev(sizeof(struct kvaser_pciefd_can),
969-
KVASER_PCIEFD_CAN_TX_MAX_COUNT);
969+
roundup_pow_of_two(KVASER_PCIEFD_CAN_TX_MAX_COUNT));
970970
if (!netdev)
971971
return -ENOMEM;
972972

@@ -995,7 +995,6 @@ static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie)
995995
can->tx_max_count = min(KVASER_PCIEFD_CAN_TX_MAX_COUNT, tx_nr_packets_max - 1);
996996

997997
can->can.clock.freq = pcie->freq;
998-
can->can.echo_skb_max = roundup_pow_of_two(can->tx_max_count);
999998
spin_lock_init(&can->lock);
1000999

10011000
can->can.bittiming_const = &kvaser_pciefd_bittiming_const;

0 commit comments

Comments
 (0)