Skip to content

Commit d9c2ba6

Browse files
lumagimarckleinebudde
authored andcommitted
can: isotp: isotp_sendmsg(): fix TX state detection and wait behavior
With patch [1], isotp_poll was updated to also queue the poller in the so->wait queue, which is used for send state changes. Since the queue now also contains polling tasks that are not interested in sending, the queue fill state can no longer be used as an indication of send readiness. As a consequence, nonblocking writes can lead to a race and lock-up of the socket if there is a second task polling the socket in parallel. With this patch, isotp_sendmsg does not consult wq_has_sleepers but instead tries to atomically set so->tx.state and waits on so->wait if it is unable to do so. This behavior is in alignment with isotp_poll, which also checks so->tx.state to determine send readiness. V2: - Revert direct exit to goto err_event_drop [1] https://lore.kernel.org/all/[email protected] Reported-by: Maxime Jayat <[email protected]> Closes: https://lore.kernel.org/linux-can/[email protected]/ Signed-off-by: Lukas Magel <[email protected]> Reviewed-by: Oliver Hartkopp <[email protected]> Fixes: 79e19fa ("can: isotp: isotp_ops: fix poll() to not report false EPOLLOUT events") Link: pylessard/python-udsoncan#178 (comment) Link: https://lore.kernel.org/all/[email protected] Signed-off-by: Marc Kleine-Budde <[email protected]>
1 parent d0f9589 commit d9c2ba6

File tree

1 file changed

+8
-11
lines changed

1 file changed

+8
-11
lines changed

net/can/isotp.c

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -948,21 +948,18 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
948948
if (!so->bound || so->tx.state == ISOTP_SHUTDOWN)
949949
return -EADDRNOTAVAIL;
950950

951-
wait_free_buffer:
952-
/* we do not support multiple buffers - for now */
953-
if (wq_has_sleeper(&so->wait) && (msg->msg_flags & MSG_DONTWAIT))
954-
return -EAGAIN;
951+
while (cmpxchg(&so->tx.state, ISOTP_IDLE, ISOTP_SENDING) != ISOTP_IDLE) {
952+
/* we do not support multiple buffers - for now */
953+
if (msg->msg_flags & MSG_DONTWAIT)
954+
return -EAGAIN;
955955

956-
/* wait for complete transmission of current pdu */
957-
err = wait_event_interruptible(so->wait, so->tx.state == ISOTP_IDLE);
958-
if (err)
959-
goto err_event_drop;
960-
961-
if (cmpxchg(&so->tx.state, ISOTP_IDLE, ISOTP_SENDING) != ISOTP_IDLE) {
962956
if (so->tx.state == ISOTP_SHUTDOWN)
963957
return -EADDRNOTAVAIL;
964958

965-
goto wait_free_buffer;
959+
/* wait for complete transmission of current pdu */
960+
err = wait_event_interruptible(so->wait, so->tx.state == ISOTP_IDLE);
961+
if (err)
962+
goto err_event_drop;
966963
}
967964

968965
/* PDU size > default => try max_pdu_size */

0 commit comments

Comments
 (0)