Skip to content

Commit 3ea5664

Browse files
hartkoppmarckleinebudde
authored andcommitted
can: isotp: sanitize CAN ID checks in isotp_bind()
Syzbot created an environment that lead to a state machine status that can not be reached with a compliant CAN ID address configuration. The provided address information consisted of CAN ID 0x6000001 and 0xC28001 which both boil down to 11 bit CAN IDs 0x001 in sending and receiving. Sanitize the SFF/EFF CAN ID values before performing the address checks. Fixes: e057dd3 ("can: add ISO 15765-2:2016 transport protocol") Link: https://lore.kernel.org/all/[email protected] Reported-by: [email protected] Signed-off-by: Oliver Hartkopp <[email protected]> Signed-off-by: Marc Kleine-Budde <[email protected]>
1 parent 231fdac commit 3ea5664

File tree

1 file changed

+21
-20
lines changed

1 file changed

+21
-20
lines changed

net/can/isotp.c

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1148,15 +1148,26 @@ static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len)
11481148
struct net *net = sock_net(sk);
11491149
int ifindex;
11501150
struct net_device *dev;
1151+
canid_t tx_id, rx_id;
11511152
int err = 0;
11521153
int notify_enetdown = 0;
11531154
int do_rx_reg = 1;
11541155

11551156
if (len < ISOTP_MIN_NAMELEN)
11561157
return -EINVAL;
11571158

1158-
if (addr->can_addr.tp.tx_id & (CAN_ERR_FLAG | CAN_RTR_FLAG))
1159-
return -EADDRNOTAVAIL;
1159+
/* sanitize tx/rx CAN identifiers */
1160+
tx_id = addr->can_addr.tp.tx_id;
1161+
if (tx_id & CAN_EFF_FLAG)
1162+
tx_id &= (CAN_EFF_FLAG | CAN_EFF_MASK);
1163+
else
1164+
tx_id &= CAN_SFF_MASK;
1165+
1166+
rx_id = addr->can_addr.tp.rx_id;
1167+
if (rx_id & CAN_EFF_FLAG)
1168+
rx_id &= (CAN_EFF_FLAG | CAN_EFF_MASK);
1169+
else
1170+
rx_id &= CAN_SFF_MASK;
11601171

11611172
if (!addr->can_ifindex)
11621173
return -ENODEV;
@@ -1168,21 +1179,13 @@ static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len)
11681179
do_rx_reg = 0;
11691180

11701181
/* do not validate rx address for functional addressing */
1171-
if (do_rx_reg) {
1172-
if (addr->can_addr.tp.rx_id == addr->can_addr.tp.tx_id) {
1173-
err = -EADDRNOTAVAIL;
1174-
goto out;
1175-
}
1176-
1177-
if (addr->can_addr.tp.rx_id & (CAN_ERR_FLAG | CAN_RTR_FLAG)) {
1178-
err = -EADDRNOTAVAIL;
1179-
goto out;
1180-
}
1182+
if (do_rx_reg && rx_id == tx_id) {
1183+
err = -EADDRNOTAVAIL;
1184+
goto out;
11811185
}
11821186

11831187
if (so->bound && addr->can_ifindex == so->ifindex &&
1184-
addr->can_addr.tp.rx_id == so->rxid &&
1185-
addr->can_addr.tp.tx_id == so->txid)
1188+
rx_id == so->rxid && tx_id == so->txid)
11861189
goto out;
11871190

11881191
dev = dev_get_by_index(net, addr->can_ifindex);
@@ -1206,16 +1209,14 @@ static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len)
12061209
ifindex = dev->ifindex;
12071210

12081211
if (do_rx_reg) {
1209-
can_rx_register(net, dev, addr->can_addr.tp.rx_id,
1210-
SINGLE_MASK(addr->can_addr.tp.rx_id),
1212+
can_rx_register(net, dev, rx_id, SINGLE_MASK(rx_id),
12111213
isotp_rcv, sk, "isotp", sk);
12121214

12131215
/* no consecutive frame echo skb in flight */
12141216
so->cfecho = 0;
12151217

12161218
/* register for echo skb's */
1217-
can_rx_register(net, dev, addr->can_addr.tp.tx_id,
1218-
SINGLE_MASK(addr->can_addr.tp.tx_id),
1219+
can_rx_register(net, dev, tx_id, SINGLE_MASK(tx_id),
12191220
isotp_rcv_echo, sk, "isotpe", sk);
12201221
}
12211222

@@ -1239,8 +1240,8 @@ static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len)
12391240

12401241
/* switch to new settings */
12411242
so->ifindex = ifindex;
1242-
so->rxid = addr->can_addr.tp.rx_id;
1243-
so->txid = addr->can_addr.tp.tx_id;
1243+
so->rxid = rx_id;
1244+
so->txid = tx_id;
12441245
so->bound = 1;
12451246

12461247
out:

0 commit comments

Comments
 (0)