Skip to contentSkip to conversation

can: isotp: fix error path in isotp_sendmsg() to unlock wait queue

PATCH In review Oliver Hartkopp 2 participants 2 messages Feb 13, 2022 – Feb 14, 2022 Stable branch

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 can: isotp: fix error path in isotp_sendmsg() to unlock wait queue

Upstream commit 8375dfac4f68

Commit 43a08c3bdac4 ("can: isotp: isotp_sendmsg(): fix TX buffer concurrent access in isotp_sendmsg()") introduced a new locking scheme that may render the userspace application in a locking state when an error is detected. This issue shows up under high load on simultaneously running isotp channels with identical configuration which is against the ISO specification and therefore breaks any reasonable PDU communication anyway.

Fixes 43a08c3bdac4 ("can: isotp: isotp_sendmsg(): fix TX buffer concurrent access in isotp_sendmsg()")
Cc Ziyang Xuan <[email protected]>
---
 net/can/isotp.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)
net/can/isotp.c +8 -5
index 53ce5b6448a5..c75de74001f9 100644
@@ -872,28 +872,28 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 			goto err_out;
 	}
 
 	if (!size || size > MAX_MSG_LENGTH) {
 		err = -EINVAL;
-		goto err_out;
+		goto err_out_drop;
 	}
 
 	err = memcpy_from_msg(so->tx.buf, msg, size);
 	if (err < 0)
-		goto err_out;
+		goto err_out_drop;
 
 	dev = dev_get_by_index(sock_net(sk), so->ifindex);
 	if (!dev) {
 		err = -ENXIO;
-		goto err_out;
+		goto err_out_drop;
 	}
 
 	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);
-		goto err_out;
+		goto err_out_drop;
 	}
 
 	can_skb_reserve(skb);
 	can_skb_prv(skb)->ifindex = dev->ifindex;
 	can_skb_prv(skb)->skbcnt = 0;
@@ -954,11 +954,11 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 	err = can_send(skb, 1);
 	dev_put(dev);
 	if (err) {
 		pr_notice_once("can-isotp: %s: can_send_ret %d\n",
 			       __func__, err);
-		goto err_out;
+		goto err_out_drop;
 	}
 
 	if (wait_tx_done) {
 		/* wait for complete transmission of current pdu */
 		wait_event_interruptible(so->wait, so->tx.state == ISOTP_IDLE);
@@ -967,10 +967,13 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
 			return -sk->sk_err;
 	}
 
 	return size;
 
+err_out_drop:
+	/* drop this PDU and unlock a potential wait queue */
+	old_state = ISOTP_IDLE;
 err_out:
 	so->tx.state = old_state;
 	if (so->tx.state == ISOTP_IDLE)
 		wake_up_interruptible(&so->wait);
 
Signature · 1 line
2.30.2
maintainer
#1 lore raw
Quoted 2 lines · Oliver Hartkopp

Upstream commit 8375dfac4f68

Now queued up, thanks.

greg k-h