Skip to content

Commit 62ec41f

Browse files
hartkoppmarckleinebudde
authored andcommitted
can: bcm: validate frame length in bcm_rx_setup() for RTR replies
bcm_tx_setup() validates cf->len against the CAN/CAN FD DLC limits before installing frames for TX_SETUP, but bcm_rx_setup() never did the same for the RTR-reply frame configured via RX_SETUP with RX_RTR_FRAME. Fixes: ffd980f ("[CAN]: Add broadcast manager (bcm) protocol") Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net> Link: https://patch.msgid.link/20260714-bcm_fixes-v15-7-562f7e3e42da@hartkopp.net Cc: stable@kernel.org Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
1 parent 12ce799 commit 62ec41f

1 file changed

Lines changed: 41 additions & 18 deletions

File tree

net/can/bcm.c

Lines changed: 41 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1246,22 +1246,37 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
12461246
return err;
12471247
}
12481248

1249-
static void bcm_rx_setup_rtr_check(struct bcm_msg_head *msg_head,
1250-
struct bcm_op *op, void *new_frames)
1249+
static int bcm_rx_setup_rtr_check(struct bcm_msg_head *msg_head,
1250+
struct bcm_op *op, void *new_frames)
12511251
{
1252+
struct canfd_frame *frame0 = new_frames;
1253+
1254+
if (!(msg_head->flags & RX_RTR_FRAME))
1255+
return 0;
1256+
1257+
/* this frame is sent out as-is by bcm_can_tx() whenever a matching
1258+
* remote request is received, so validate its length the same way
1259+
* bcm_tx_setup() validates TX_SETUP frames before installing it
1260+
*/
1261+
if (msg_head->flags & CAN_FD_FRAME) {
1262+
if (frame0->len > 64)
1263+
return -EINVAL;
1264+
} else {
1265+
if (frame0->len > 8)
1266+
return -EINVAL;
1267+
}
1268+
12521269
/* funny feature in RX(!)_SETUP only for RTR-mode:
12531270
* copy can_id into frame BUT without RTR-flag to
12541271
* prevent a full-load-loopback-test ... ;-]
12551272
* normalize this on the staged buffer, before it is
12561273
* ever installed into op->frames.
12571274
*/
1258-
if (msg_head->flags & RX_RTR_FRAME) {
1259-
struct canfd_frame *frame0 = new_frames;
1275+
if ((msg_head->flags & TX_CP_CAN_ID) ||
1276+
frame0->can_id == op->can_id)
1277+
frame0->can_id = op->can_id & ~CAN_RTR_FLAG;
12601278

1261-
if ((msg_head->flags & TX_CP_CAN_ID) ||
1262-
frame0->can_id == op->can_id)
1263-
frame0->can_id = op->can_id & ~CAN_RTR_FLAG;
1264-
}
1279+
return 0;
12651280
}
12661281

12671282
/*
@@ -1324,7 +1339,11 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
13241339
return err;
13251340
}
13261341

1327-
bcm_rx_setup_rtr_check(msg_head, op, new_frames);
1342+
err = bcm_rx_setup_rtr_check(msg_head, op, new_frames);
1343+
if (err < 0) {
1344+
kfree(new_frames);
1345+
return err;
1346+
}
13281347
}
13291348

13301349
spin_lock_bh(&op->bcm_rx_update_lock);
@@ -1397,16 +1416,12 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
13971416
if (msg_head->nframes) {
13981417
err = memcpy_from_msg(op->frames, msg,
13991418
msg_head->nframes * op->cfsiz);
1400-
if (err < 0) {
1401-
if (op->frames != &op->sframe)
1402-
kfree(op->frames);
1403-
if (op->last_frames != &op->last_sframe)
1404-
kfree(op->last_frames);
1405-
kfree(op);
1406-
return err;
1407-
}
1419+
if (err < 0)
1420+
goto free_op;
14081421

1409-
bcm_rx_setup_rtr_check(msg_head, op, op->frames);
1422+
err = bcm_rx_setup_rtr_check(msg_head, op, op->frames);
1423+
if (err < 0)
1424+
goto free_op;
14101425
}
14111426

14121427
/* bcm_can_tx / bcm_tx_timeout_handler needs this */
@@ -1505,6 +1520,14 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
15051520
}
15061521

15071522
return msg_head->nframes * op->cfsiz + MHSIZ;
1523+
1524+
free_op:
1525+
if (op->frames != &op->sframe)
1526+
kfree(op->frames);
1527+
if (op->last_frames != &op->last_sframe)
1528+
kfree(op->last_frames);
1529+
kfree(op);
1530+
return err;
15081531
}
15091532

15101533
/*

0 commit comments

Comments
 (0)