Commit 2b17c400 authored by Norbert Slusarek's avatar Norbert Slusarek Committed by Marc Kleine-Budde
Browse files

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

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.

Fixes: 921ca574 ("can: isotp: add SF_BROADCAST support for functional addressing")
Link: https://lore.kernel.org/r/trinity-e6ae9efa-9afb-4326-84c0-f3609b9b8168-1620773528307@3c-app-gmx-bs06


Reported-by: default avatarNorbert Slusarek <nslusarek@gmx.net>
Signed-off-by: default avatarThadeu Lima de Souza Cascardo <cascardo@canonical.com>
Signed-off-by: default avatarNorbert Slusarek <nslusarek@gmx.net>
Acked-by: default avatarOliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: default avatarMarc Kleine-Budde <mkl@pengutronix.de>
parent 440c3247

net/can/isotp.c

+33 −16
Original line number Diff line number Diff line
@@ -1062,26 +1062,30 @@ 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;



	if (!addr->can_ifindex)

		return -ENODEV;



	lock_sock(sk);



	/* do not register frame reception for functional addressing */

	if (so->opt.flags & CAN_ISOTP_SF_BROADCAST)

		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)

			return -EADDRNOTAVAIL;



		if (addr->can_addr.tp.rx_id & (CAN_ERR_FLAG | CAN_RTR_FLAG))

			return -EADDRNOTAVAIL;

		if (addr->can_addr.tp.rx_id == addr->can_addr.tp.tx_id) {

			err = -EADDRNOTAVAIL;

			goto out;

		}



	if (addr->can_addr.tp.tx_id & (CAN_ERR_FLAG | CAN_RTR_FLAG))

		return -EADDRNOTAVAIL;



	if (!addr->can_ifindex)

		return -ENODEV;



	lock_sock(sk);

		if (addr->can_addr.tp.rx_id & (CAN_ERR_FLAG | CAN_RTR_FLAG)) {

			err = -EADDRNOTAVAIL;

			goto out;

		}

	}



	if (so->bound && addr->can_ifindex == so->ifindex &&

	    addr->can_addr.tp.rx_id == so->rxid &&
@@ -1164,16 +1168,13 @@ static int isotp_getname(struct socket *sock, struct sockaddr *uaddr, int peer) 
	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;
@@ -1248,6 +1249,22 @@ 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)

{