From: Ziyang Xuan william.xuanziyang@huawei.com
commit 43a08c3bdac4cb42eff8fe5e2278bffe0c5c3daa upstream.
When isotp_sendmsg() concurrent, tx.state of all TX processes can be ISOTP_IDLE. The conditions so->tx.state != ISOTP_IDLE and wq_has_sleeper(&so->wait) can not protect TX buffer from being accessed by multiple TX processes.
We can use cmpxchg() to try to modify tx.state to ISOTP_SENDING firstly. If the modification of the previous process succeed, the later process must wait tx.state to ISOTP_IDLE firstly. Thus, we can ensure TX buffer is accessed by only one process at the same time. And we should also restore the original tx.state at the subsequent error processes.
Fixes: e057dd3fc20f ("can: add ISO 15765-2:2016 transport protocol") Link: https://lore.kernel.org/all/c2517874fbdf4188585cf9ddf67a8fa74d5dbde5.1633764... Cc: stable@vger.kernel.org Signed-off-by: Ziyang Xuan william.xuanziyang@huawei.com Acked-by: Oliver Hartkopp socketcan@hartkopp.net Signed-off-by: Marc Kleine-Budde mkl@pengutronix.de Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org --- net/can/isotp.c | 40 +++++++++++++++++++++++++++------------- 1 file changed, 27 insertions(+), 13 deletions(-)
--- a/net/can/isotp.c +++ b/net/can/isotp.c @@ -121,7 +121,7 @@ enum { struct tpcon { int idx; int len; - u8 state; + u32 state; u8 bs; u8 sn; u8 ll_dl; @@ -846,6 +846,7 @@ static int isotp_sendmsg(struct socket * { struct sock *sk = sock->sk; struct isotp_sock *so = isotp_sk(sk); + u32 old_state = so->tx.state; struct sk_buff *skb; struct net_device *dev; struct canfd_frame *cf; @@ -858,39 +859,45 @@ static int isotp_sendmsg(struct socket * return -EADDRNOTAVAIL;
/* we do not support multiple buffers - for now */ - if (so->tx.state != ISOTP_IDLE || wq_has_sleeper(&so->wait)) { - if (msg->msg_flags & MSG_DONTWAIT) - return -EAGAIN; + if (cmpxchg(&so->tx.state, ISOTP_IDLE, ISOTP_SENDING) != ISOTP_IDLE || + wq_has_sleeper(&so->wait)) { + if (msg->msg_flags & MSG_DONTWAIT) { + err = -EAGAIN; + goto err_out; + }
/* wait for complete transmission of current pdu */ err = wait_event_interruptible(so->wait, so->tx.state == ISOTP_IDLE); if (err) - return err; + goto err_out; }
- if (!size || size > MAX_MSG_LENGTH) - return -EINVAL; + if (!size || size > MAX_MSG_LENGTH) { + err = -EINVAL; + goto err_out; + }
err = memcpy_from_msg(so->tx.buf, msg, size); if (err < 0) - return err; + goto err_out;
dev = dev_get_by_index(sock_net(sk), so->ifindex); - if (!dev) - return -ENXIO; + if (!dev) { + err = -ENXIO; + goto err_out; + }
skb = sock_alloc_send_skb(sk, so->ll.mtu + sizeof(struct can_skb_priv), msg->msg_flags & MSG_DONTWAIT, &err); if (!skb) { dev_put(dev); - return err; + goto err_out; }
can_skb_reserve(skb); can_skb_prv(skb)->ifindex = dev->ifindex; can_skb_prv(skb)->skbcnt = 0;
- so->tx.state = ISOTP_SENDING; so->tx.len = size; so->tx.idx = 0;
@@ -949,7 +956,7 @@ static int isotp_sendmsg(struct socket * if (err) { pr_notice_once("can-isotp: %s: can_send_ret %d\n", __func__, err); - return err; + goto err_out; }
if (wait_tx_done) { @@ -961,6 +968,13 @@ static int isotp_sendmsg(struct socket * }
return size; + +err_out: + so->tx.state = old_state; + if (so->tx.state == ISOTP_IDLE) + wake_up_interruptible(&so->wait); + + return err; }
static int isotp_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,