Skip to contentSkip to conversation

can: isotp: prevent race between isotp_bind() and isotp_setsockopt()

PATCH In review Oliver Hartkopp 2 participants 3 messages Feb 16, 2022 – Feb 17, 2022 Stable branch

lore
Share & tools
lore
#0 lore raw

PATCH 1/2 can: isotp: prevent race between isotp_bind() and isotp_setsockopt()

Upstream commit 2b17c400aeb44daf041627722581ade527bb3c1d

The fixes tag of the uptream patch points to commit 921ca574cd38 ("can:
isotp: add SF_BROADCAST support for functional addressing") which showed up in Linux 5.11 but the described issue already existed in Linux 5.10.

Norbert Slusarek writes:

A race condition was found in isotp_setsockopt() which allows to change socket options after the socket was bound.
For the specific case of SF_BROADCAST support, this might lead to possible use-after-free because can_rx_unregister() is not called.

Checking for the flag under the socket lock in isotp_bind() and taking the lock in isotp_setsockopt() fixes the issue.

---
 net/can/isotp.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)
net/can/isotp.c +17 -4
index 37db4d232313..3f11d2b314b6 100644
@@ -1191,20 +1191,17 @@ static int isotp_getname(struct socket *sock, struct sockaddr *uaddr, int peer)
 	addr->can_addr.tp.tx_id = so->txid;
 
 	return ISOTP_MIN_NAMELEN;
 }
 
-static int isotp_setsockopt(struct socket *sock, int level, int optname,
+static int isotp_setsockopt_locked(struct socket *sock, int level, int optname,
 			    sockptr_t optval, unsigned int optlen)
 {
 	struct sock *sk = sock->sk;
 	struct isotp_sock *so = isotp_sk(sk);
 	int ret = 0;
 
-	if (level != SOL_CAN_ISOTP)
-		return -EINVAL;
-
 	if (so->bound)
 		return -EISCONN;
 
 	switch (optname) {
 	case CAN_ISOTP_OPTS:
@@ -1275,10 +1272,26 @@ static int isotp_setsockopt(struct socket *sock, int level, int optname,
 	}
 
 	return ret;
 }
 
+static int isotp_setsockopt(struct socket *sock, int level, int optname,
+			    sockptr_t optval, unsigned int optlen)
+
+{
+	struct sock *sk = sock->sk;
+	int ret;
+
+	if (level != SOL_CAN_ISOTP)
+		return -EINVAL;
+
+	lock_sock(sk);
+	ret = isotp_setsockopt_locked(sock, level, optname, optval, optlen);
+	release_sock(sk);
+	return ret;
+}
+
 static int isotp_getsockopt(struct socket *sock, int level, int optname,
 			    char __user *optval, int __user *optlen)
 {
 	struct sock *sk = sock->sk;
 	struct isotp_sock *so = isotp_sk(sk);
Signature · 1 line
2.30.2
#1 lore raw

PATCH 2/2 can: isotp: add SF_BROADCAST support for functional addressing

Upstream commit 921ca574cd382142add8b12d0a7117f495510de5

The patch was intended for 5.10 but missed the merge window by some days. This missing patch continously breaks the backport of stable fixes and is the only missing feature of upstream isotp in Linux 5.10 e.g. for RasPi.

When CAN_ISOTP_SF_BROADCAST is set in the CAN_ISOTP_OPTS flags the CAN_ISOTP socket is switched into functional addressing mode, where only single frame (SF) protocol data units can be send on the specified CAN interface and the given tp.tx_id after bind().

In opposite to normal and extended addressing this socket does not register a CAN-ID for reception which would be needed for a 1-to-1 ISOTP connection with a segmented bi-directional data transfer.

Sending SFs on this socket is therefore a TX-only 'broadcast' operation.

---
 include/uapi/linux/can/isotp.h |  2 +-
 net/can/isotp.c                | 50 ++++++++++++++++++++++++----------
 2 files changed, 37 insertions(+), 15 deletions(-)
include/uapi/linux/can/isotp.h +1 -1
index 7793b26aa154..c55935b64ccc 100644
@@ -133,11 +133,11 @@ struct can_isotp_ll_options {
 #define CAN_ISOTP_HALF_DUPLEX	0x040	/* half duplex error state handling */
 #define CAN_ISOTP_FORCE_TXSTMIN	0x080	/* ignore stmin from received FC */
 #define CAN_ISOTP_FORCE_RXSTMIN	0x100	/* ignore CFs depending on rx stmin */
 #define CAN_ISOTP_RX_EXT_ADDR	0x200	/* different rx extended addressing */
 #define CAN_ISOTP_WAIT_TX_DONE	0x400	/* wait for tx completion */
-
+#define CAN_ISOTP_SF_BROADCAST	0x800	/* 1-to-N functional addressing */
 
 /* default values */
 
 #define CAN_ISOTP_DEFAULT_FLAGS		0
 #define CAN_ISOTP_DEFAULT_EXT_ADDRESS	0x00
net/can/isotp.c +36 -14

124 lines not on this page · raw message

Signature · 1 line
2.30.2
maintainer
#2 lore raw

Re: [BACKPORT stable Linux-5.10.y 2/2] can: isotp: add SF_BROADCAST support for functional addressing

Quoted 17 lines · Oliver Hartkopp

Upstream commit 921ca574cd382142add8b12d0a7117f495510de5

The patch was intended for 5.10 but missed the merge window by some days. This missing patch continously breaks the backport of stable fixes and is the only missing feature of upstream isotp in Linux 5.10 e.g. for RasPi.

When CAN_ISOTP_SF_BROADCAST is set in the CAN_ISOTP_OPTS flags the CAN_ISOTP socket is switched into functional addressing mode, where only single frame (SF) protocol data units can be send on the specified CAN interface and the given tp.tx_id after bind().

In opposite to normal and extended addressing this socket does not register a CAN-ID for reception which would be needed for a 1-to-1 ISOTP connection with a segmented bi-directional data transfer.

Sending SFs on this socket is therefore a TX-only 'broadcast' operation.

Quoted 8 lines
---
 include/uapi/linux/can/isotp.h |  2 +-
 net/can/isotp.c                | 50 ++++++++++++++++++++++++----------
 2 files changed, 37 insertions(+), 15 deletions(-)

Both now queued up, thanks.

greg k-h