From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: stable@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
patches@lists.linux.dev, "linux-can@vger.kernel.org,
lukas.magel@posteo.net, patches@lists.linux.dev,
maxime.jayat@mobile-devices.fr, mkl@pengutronix.de,
michal.sojka@cvut.cz, Oliver Hartkopp" <socketcan@hartkopp.net>,
Maxime Jayat <maxime.jayat@mobile-devices.fr>,
Lukas Magel <lukas.magel@posteo.net>,
Marc Kleine-Budde <mkl@pengutronix.de>,
Sasha Levin <sashal@kernel.org>,
Oliver Hartkopp <socketcan@hartkopp.net>
Subject: [PATCH 5.15 110/128] can: isotp: isotp_sendmsg(): fix TX state detection and wait behavior
Date: Mon, 6 Nov 2023 14:04:30 +0100 [thread overview]
Message-ID: <20231106130314.174525847@linuxfoundation.org> (raw)
In-Reply-To: <20231106130309.112650042@linuxfoundation.org>
5.15-stable review patch. If anyone has any objections, please let me know.
------------------
From: Oliver Hartkopp <socketcan@hartkopp.net>
From: Lukas Magel <lukas.magel@posteo.net>
[ Upstream commit d9c2ba65e651467de739324d978b04ed8729f483 ]
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/20230331125511.372783-1-michal.sojka@cvut.cz
Reported-by: Maxime Jayat <maxime.jayat@mobile-devices.fr>
Closes: https://lore.kernel.org/linux-can/11328958-453f-447f-9af8-3b5824dfb041@munic.io/
Signed-off-by: Lukas Magel <lukas.magel@posteo.net>
Reviewed-by: Oliver Hartkopp <socketcan@hartkopp.net>
Fixes: 79e19fa79cb5 ("can: isotp: isotp_ops: fix poll() to not report false EPOLLOUT events")
Link: https://github.com/pylessard/python-udsoncan/issues/178#issuecomment-1743786590
Link: https://lore.kernel.org/all/20230827092205.7908-1-lukas.magel@posteo.net
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
net/can/isotp.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -925,21 +925,18 @@ static int isotp_sendmsg(struct socket *
if (!so->bound || so->tx.state == ISOTP_SHUTDOWN)
return -EADDRNOTAVAIL;
-wait_free_buffer:
- /* we do not support multiple buffers - for now */
- if (wq_has_sleeper(&so->wait) && (msg->msg_flags & MSG_DONTWAIT))
- return -EAGAIN;
+ while (cmpxchg(&so->tx.state, ISOTP_IDLE, ISOTP_SENDING) != ISOTP_IDLE) {
+ /* we do not support multiple buffers - for now */
+ if (msg->msg_flags & MSG_DONTWAIT)
+ return -EAGAIN;
- /* wait for complete transmission of current pdu */
- err = wait_event_interruptible(so->wait, so->tx.state == ISOTP_IDLE);
- if (err)
- goto err_event_drop;
-
- if (cmpxchg(&so->tx.state, ISOTP_IDLE, ISOTP_SENDING) != ISOTP_IDLE) {
if (so->tx.state == ISOTP_SHUTDOWN)
return -EADDRNOTAVAIL;
- goto wait_free_buffer;
+ /* wait for complete transmission of current pdu */
+ err = wait_event_interruptible(so->wait, so->tx.state == ISOTP_IDLE);
+ if (err)
+ goto err_event_drop;
}
if (!size || size > MAX_MSG_LENGTH) {
prev parent reply other threads:[~2023-11-06 13:28 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20231106130309.112650042@linuxfoundation.org>
2023-11-06 13:04 ` [PATCH 5.15 104/128] can: isotp: set max PDU size to 64 kByte Greg Kroah-Hartman
2023-11-06 13:04 ` [PATCH 5.15 107/128] can: isotp: handle wait_event_interruptible() return values Greg Kroah-Hartman
2023-11-06 13:04 ` [PATCH 5.15 108/128] can: isotp: add local echo tx processing and tx without FC Greg Kroah-Hartman
2023-11-06 13:04 ` [PATCH 5.15 109/128] can: isotp: isotp_bind(): do not validate unused address information Greg Kroah-Hartman
2023-11-06 13:04 ` Greg Kroah-Hartman [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231106130314.174525847@linuxfoundation.org \
--to=gregkh@linuxfoundation.org \
--cc=lukas.magel@posteo.net \
--cc=maxime.jayat@mobile-devices.fr \
--cc=mkl@pengutronix.de \
--cc=patches@lists.linux.dev \
--cc=sashal@kernel.org \
--cc=socketcan@hartkopp.net \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).