diff mbox series

[net-next,1/5] can: isotp: sanitize CAN ID checks in isotp_bind()

Message ID 20220316204710.716341-2-mkl@pengutronix.de (mailing list archive)
State Accepted
Commit 3ea566422cbde9610c2734980d1286ab681bb40e
Delegated to: Netdev Maintainers
Headers show
Series [net-next,1/5] can: isotp: sanitize CAN ID checks in isotp_bind() | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Pull request is its own cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 6 of 6 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 80 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Marc Kleine-Budde March 16, 2022, 8:47 p.m. UTC 
From: Oliver Hartkopp <socketcan@hartkopp.net>

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: e057dd3fc20f ("can: add ISO 15765-2:2016 transport protocol")
Link: https://lore.kernel.org/all/20220316164258.54155-1-socketcan@hartkopp.net
Reported-by: syzbot+2339c27f5c66c652843e@syzkaller.appspotmail.com
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 net/can/isotp.c | 41 +++++++++++++++++++++--------------------
 1 file changed, 21 insertions(+), 20 deletions(-)


base-commit: 231fdac3e58f4e52e387930c73bf535439607563

Comments

patchwork-bot+netdevbpf@kernel.org March 17, 2022, 2 a.m. UTC | #1 
Hello:

This series was applied to netdev/net-next.git (master)
by Marc Kleine-Budde <mkl@pengutronix.de>:

On Wed, 16 Mar 2022 21:47:06 +0100 you wrote:
> From: Oliver Hartkopp <socketcan@hartkopp.net>
> 
> 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.
> 
> [...]

Here is the summary with links:
  - [net-next,1/5] can: isotp: sanitize CAN ID checks in isotp_bind()
    https://git.kernel.org/netdev/net-next/c/3ea566422cbd
  - [net-next,2/5] can: isotp: return -EADDRNOTAVAIL when reading from unbound socket
    https://git.kernel.org/netdev/net-next/c/30ffd5332e06
  - [net-next,3/5] can: isotp: support MSG_TRUNC flag when reading from socket
    https://git.kernel.org/netdev/net-next/c/42bf50a1795a
  - [net-next,4/5] dt-bindings: can: xilinx_can: Convert Xilinx CAN binding to YAML
    https://git.kernel.org/netdev/net-next/c/7843d3c8e5e6
  - [net-next,5/5] can: ucan: fix typos in comments
    https://git.kernel.org/netdev/net-next/c/c34983c94166

You are awesome, thank you!
diff mbox series

Patch

diff --git a/net/can/isotp.c b/net/can/isotp.c
index d4c0b4704987..1662103ce125 100644
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -1148,6 +1148,7 @@  static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len)
 	struct net *net = sock_net(sk);
 	int ifindex;
 	struct net_device *dev;
+	canid_t tx_id, rx_id;
 	int err = 0;
 	int notify_enetdown = 0;
 	int do_rx_reg = 1;
@@ -1155,8 +1156,18 @@  static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len)
 	if (len < ISOTP_MIN_NAMELEN)
 		return -EINVAL;
 
-	if (addr->can_addr.tp.tx_id & (CAN_ERR_FLAG | CAN_RTR_FLAG))
-		return -EADDRNOTAVAIL;
+	/* sanitize tx/rx CAN identifiers */
+	tx_id = addr->can_addr.tp.tx_id;
+	if (tx_id & CAN_EFF_FLAG)
+		tx_id &= (CAN_EFF_FLAG | CAN_EFF_MASK);
+	else
+		tx_id &= CAN_SFF_MASK;
+
+	rx_id = addr->can_addr.tp.rx_id;
+	if (rx_id & CAN_EFF_FLAG)
+		rx_id &= (CAN_EFF_FLAG | CAN_EFF_MASK);
+	else
+		rx_id &= CAN_SFF_MASK;
 
 	if (!addr->can_ifindex)
 		return -ENODEV;
@@ -1168,21 +1179,13 @@  static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len)
 		do_rx_reg = 0;
 
 	/* do not validate rx address for functional addressing */
-	if (do_rx_reg) {
-		if (addr->can_addr.tp.rx_id == addr->can_addr.tp.tx_id) {
-			err = -EADDRNOTAVAIL;
-			goto out;
-		}
-
-		if (addr->can_addr.tp.rx_id & (CAN_ERR_FLAG | CAN_RTR_FLAG)) {
-			err = -EADDRNOTAVAIL;
-			goto out;
-		}
+	if (do_rx_reg && rx_id == tx_id) {
+		err = -EADDRNOTAVAIL;
+		goto out;
 	}
 
 	if (so->bound && addr->can_ifindex == so->ifindex &&
-	    addr->can_addr.tp.rx_id == so->rxid &&
-	    addr->can_addr.tp.tx_id == so->txid)
+	    rx_id == so->rxid && tx_id == so->txid)
 		goto out;
 
 	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)
 	ifindex = dev->ifindex;
 
 	if (do_rx_reg) {
-		can_rx_register(net, dev, addr->can_addr.tp.rx_id,
-				SINGLE_MASK(addr->can_addr.tp.rx_id),
+		can_rx_register(net, dev, rx_id, SINGLE_MASK(rx_id),
 				isotp_rcv, sk, "isotp", sk);
 
 		/* no consecutive frame echo skb in flight */
 		so->cfecho = 0;
 
 		/* register for echo skb's */
-		can_rx_register(net, dev, addr->can_addr.tp.tx_id,
-				SINGLE_MASK(addr->can_addr.tp.tx_id),
+		can_rx_register(net, dev, tx_id, SINGLE_MASK(tx_id),
 				isotp_rcv_echo, sk, "isotpe", sk);
 	}
 
@@ -1239,8 +1240,8 @@  static int isotp_bind(struct socket *sock, struct sockaddr *uaddr, int len)
 
 	/* switch to new settings */
 	so->ifindex = ifindex;
-	so->rxid = addr->can_addr.tp.rx_id;
-	so->txid = addr->can_addr.tp.tx_id;
+	so->rxid = rx_id;
+	so->txid = tx_id;
 	so->bound = 1;
 
 out: