Skip to content

Commit d72866a

Browse files
hartkoppgregkh
authored andcommitted
can: isotp: sanitize CAN ID checks in isotp_bind()
commit 3ea5664 upstream. 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]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent fde8c5c commit d72866a

File tree

1 file changed

+20
-18
lines changed

1 file changed

+20
-18
lines changed

net/can/isotp.c

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1102,15 +1102,26 @@ static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len)
11021102
struct net *net = sock_net(sk);
11031103
int ifindex;
11041104
struct net_device *dev;
1105+
canid_t tx_id, rx_id;
11051106
int err = 0;
11061107
int notify_enetdown = 0;
11071108
int do_rx_reg = 1;
11081109

11091110
if (len < ISOTP_MIN_NAMELEN)
11101111
return -EINVAL;
11111112

1112-
if (addr->can_addr.tp.tx_id & (CAN_ERR_FLAG | CAN_RTR_FLAG))
1113-
return -EADDRNOTAVAIL;
1113+
/* sanitize tx/rx CAN identifiers */
1114+
tx_id = addr->can_addr.tp.tx_id;
1115+
if (tx_id & CAN_EFF_FLAG)
1116+
tx_id &= (CAN_EFF_FLAG | CAN_EFF_MASK);
1117+
else
1118+
tx_id &= CAN_SFF_MASK;
1119+
1120+
rx_id = addr->can_addr.tp.rx_id;
1121+
if (rx_id & CAN_EFF_FLAG)
1122+
rx_id &= (CAN_EFF_FLAG | CAN_EFF_MASK);
1123+
else
1124+
rx_id &= CAN_SFF_MASK;
11141125

11151126
if (!addr->can_ifindex)
11161127
return -ENODEV;
@@ -1122,21 +1133,13 @@ static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len)
11221133
do_rx_reg = 0;
11231134

11241135
/* do not validate rx address for functional addressing */
1125-
if (do_rx_reg) {
1126-
if (addr->can_addr.tp.rx_id == addr->can_addr.tp.tx_id) {
1127-
err = -EADDRNOTAVAIL;
1128-
goto out;
1129-
}
1130-
1131-
if (addr->can_addr.tp.rx_id & (CAN_ERR_FLAG | CAN_RTR_FLAG)) {
1132-
err = -EADDRNOTAVAIL;
1133-
goto out;
1134-
}
1136+
if (do_rx_reg && rx_id == tx_id) {
1137+
err = -EADDRNOTAVAIL;
1138+
goto out;
11351139
}
11361140

11371141
if (so->bound && addr->can_ifindex == so->ifindex &&
1138-
addr->can_addr.tp.rx_id == so->rxid &&
1139-
addr->can_addr.tp.tx_id == so->txid)
1142+
rx_id == so->rxid && tx_id == so->txid)
11401143
goto out;
11411144

11421145
dev = dev_get_by_index(net, addr->can_ifindex);
@@ -1160,8 +1163,7 @@ static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len)
11601163
ifindex = dev->ifindex;
11611164

11621165
if (do_rx_reg)
1163-
can_rx_register(net, dev, addr->can_addr.tp.rx_id,
1164-
SINGLE_MASK(addr->can_addr.tp.rx_id),
1166+
can_rx_register(net, dev, rx_id, SINGLE_MASK(rx_id),
11651167
isotp_rcv, sk, "isotp", sk);
11661168

11671169
dev_put(dev);
@@ -1181,8 +1183,8 @@ static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len)
11811183

11821184
/* switch to new settings */
11831185
so->ifindex = ifindex;
1184-
so->rxid = addr->can_addr.tp.rx_id;
1185-
so->txid = addr->can_addr.tp.tx_id;
1186+
so->rxid = rx_id;
1187+
so->txid = tx_id;
11861188
so->bound = 1;
11871189

11881190
out:

0 commit comments

Comments
 (0)