[PATCH net] can: can327: Fix out-of-bounds write in can327_parse_frame()
Baul Lee <[email protected]>
| Newsgroups | org.kernel.vger.linux-can,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
can327_parse_frame() assigns the CAN payload length from the DLC nibble of the adapter's ASCII frame line, hex_to_bin(elm->rxbuf[datastart - 2]), without validating it. A standard-format line only has to satisfy rxbuf[3] == ' ' and rxbuf[5] == ' ', so the DLC nibble rxbuf[4] can be a space, for which hex_to_bin() returns -1, and that becomes 255 in the u8 frame->len. A hex nibble of 9 to f is not rejected either, while CAN_MAX_DLEN is 8. frame->data[] is the 8-byte payload of the 16-byte struct can_frame returned by alloc_can_skb(), so the data-nibble loop writes up to 255 device-controlled bytes, 247 of them past the frame and over the trailing skb_shared_info. The length check before the loop only requires the line to be frame->len * 3 + datastart bytes, which a long enough line of hex and spaces satisfies. Freeing the corrupted skb then faults: pc : skb_release_data+0xf4/0x200 Call trace: skb_release_data+0xf4/0x200 (P) sk_skb_reason_drop+0x40/0xa4 can_rcv+0x6c/0xbc __netif_receive_skb_one_core+0x40/0x4c can327_ldisc_rx+0xc8/0x140 tty_ldisc_receive_buf+0x48/0x60 flush_to_ldisc+0xdc/0x1b0 Kernel panic - not syncing: Oops: Fatal exception in interrupt Reject the line when the nibble is not a hex digit or exceeds CAN_MAX_DLEN, as the parser already does for other malformed lines. Attaching the N_CAN327 line discipline requires CAP_NET_ADMIN, but the frame lines then come from the ELM327 device, so a malicious adapter reaches this path with device-controlled data. Discovered by XBOW, triaged by Baul Lee <[email protected]> Fixes: 43da2f07622f ("can: can327: CAN/ldisc driver for ELM327 based OBD-II adapters") Signed-off-by: Baul Lee <[email protected]> --- drivers/net/can/can327.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/net/can/can327.c b/drivers/net/can/can327.c index 90f5e35f3c8f..c76a6378d4d6 100644 --- a/drivers/net/can/can327.c +++ b/drivers/net/can/can327.c @@ -395,6 +395,7 @@ static int can327_parse_frame(struct can327 *elm, size_t len) struct sk_buff *skb; int hexlen; int datastart; + int dlc; int i; lockdep_assert_held(&elm->lock); @@ -460,7 +461,13 @@ static int can327_parse_frame(struct can327 *elm, size_t len) */ /* Read CAN data length */ - frame->len = (hex_to_bin(elm->rxbuf[datastart - 2]) << 0); + dlc = hex_to_bin(elm->rxbuf[datastart - 2]); + if (dlc < 0 || dlc > CAN_MAX_DLEN) { + /* Not a hex digit, or more than CAN_MAX_DLEN bytes. */ + kfree_skb(skb); + return -ENODATA; + } + frame->len = dlc; /* Read CAN ID */ if (frame->can_id & CAN_EFF_FLAG) { -- 2.50.1