From: Stanislav Fomichev sdf@fomichev.me
[ Upstream commit 18282100d7040614b553f1cad737cb689c04e2b9 ]
tcp_recvmsg_dmabuf can export the following errors: - EFAULT when linear copy fails - ETOOSMALL when cmsg put fails - ENODEV if one of the frags is readable - ENOMEM on xarray failures
But they are all ignored and replaced by EFAULT in the caller (tcp_recvmsg_locked). Expose real error to the userspace to add more transparency on what specifically fails.
In non-devmem case (skb_copy_datagram_msg) doing `if (!copied) copied=-EFAULT` is ok because skb_copy_datagram_msg can return only EFAULT.
Reviewed-by: David Ahern dsahern@kernel.org Reviewed-by: Mina Almasry almasrymina@google.com Reviewed-by: Eric Dumazet edumazet@google.com Signed-off-by: Stanislav Fomichev sdf@fomichev.me Link: https://patch.msgid.link/20250910162429.4127997-1-sdf@fomichev.me Signed-off-by: Jakub Kicinski kuba@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
YES
This is a small, contained bugfix that improves error reporting for the new MSG_SOCK_DEVMEM TCP receive path without changing normal TCP behavior. It should be backported to stable trees which already include the devmem TCP feature.
- Fix scope and change details - In the devmem path of `tcp_recvmsg_locked`, errors returned by `tcp_recvmsg_dmabuf()` were previously collapsed to `-EFAULT`. The patch changes this to expose the original error to userspace and only treat strictly negative returns as errors: - Change: `if (err < 0) { if (!copied) copied = err; break; }` and keep positive `err` as the actual bytes consumed via `used = err` (net/ipv4/tcp.c:2839–2847). - This replaces the old behavior which treated `err <= 0` as error and always returned `-EFAULT` if nothing was copied. - The non-devmem (normal) path remains unchanged and keeps mapping failures of `skb_copy_datagram_msg()` to `-EFAULT` when no data has been copied (net/ipv4/tcp.c:2819–2827). This is correct because `skb_copy_datagram_msg` can only fail with `-EFAULT`.
- Error contract and correctness - `tcp_recvmsg_dmabuf()` already distinguishes several error cases: - `-ENODEV` when a supposed devmem skb has readable frags (misconfiguration/unsupported) (net/ipv4/tcp.c:2490–2492). - `-ETOOSMALL` when control buffer is too small for CMSG via `put_cmsg_notrunc()` (net/ipv4/tcp.c:2515–2520, net/core/scm.c:311). - `-ENOMEM` on xarray allocation failures in `tcp_xa_pool_refill()` (net/ipv4/tcp.c:2567–2570). - `-EFAULT` on linear copy failures or unsatisfied `remaining_len` (net/ipv4/tcp.c:2500–2505, 2609–2612). - Return semantics ensure safety of the `< 0` check: on success, it returns the number of bytes “sent” to userspace; on error with no progress, it returns a negative errno (net/ipv4/tcp.c:2615–2619). Given the caller’s `used > 0`, a zero return from `tcp_recvmsg_dmabuf()` is not expected; switching from `<= 0` to `< 0` avoids misclassifying a non-existent zero as an error and prevents false error handling.
- Impact and risk - Behavior change is limited to sockets using `MSG_SOCK_DEVMEM`; normal TCP receive paths are unaffected. - Users now receive accurate errno values (`-ENODEV`, `-ENOMEM`, `-ETOOSMALL`, `-EFAULT`) instead of a blanket `-EFAULT`. This improves diagnosability and allows appropriate user-space handling (e.g., resizing control buffer on `-ETOOSMALL`, backing off on `-ENOMEM`, detecting misconfiguration via `-ENODEV`). - No ABI or data structure changes; no architectural alterations; code change is localized to `net/ipv4/tcp.c`. - Selftests for devmem do not assume `-EFAULT` specifically (they only treat `-EFAULT` as unrecoverable and otherwise continue), so the change does not regress the existing test expectations (tools/testing/selftests/drivers/net/hw/ncdevmem.c:940–973).
- Stable suitability - Fixes an actual bug (incorrect, lossy error propagation) that affects users of a new feature introduced recently (“tcp: RX path for devmem TCP”, commit 8f0b3cc9a4c1). - Minimal, well-scoped diff; low regression risk; no dependency churn. - Backport only to stable series that already contain the devmem TCP feature and `tcp_recvmsg_dmabuf()`; it is not applicable to older series that predate this feature.
Code references - Devmem receive error propagation fix: net/ipv4/tcp.c:2839–2847 - Non-devmem path (unchanged, still maps to -EFAULT only): net/ipv4/tcp.c:2819–2827 - `tcp_recvmsg_dmabuf()` error sources and contract: - `-ENODEV`: net/ipv4/tcp.c:2490–2492 - `-EFAULT` (linear copy): net/ipv4/tcp.c:2500–2505 - `-ETOOSMALL` via `put_cmsg_notrunc`: net/ipv4/tcp.c:2515–2520; definition returns `-ETOOSMALL`/`-EFAULT`: net/core/scm.c:311 - `-ENOMEM` via xarray: net/ipv4/tcp.c:2567–2570 - Return negative only if no bytes sent: net/ipv4/tcp.c:2615–2619
net/ipv4/tcp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c index ba36f558f144c..f421cad69d8c9 100644 --- a/net/ipv4/tcp.c +++ b/net/ipv4/tcp.c @@ -2821,9 +2821,9 @@ static int tcp_recvmsg_locked(struct sock *sk, struct msghdr *msg, size_t len,
err = tcp_recvmsg_dmabuf(sk, skb, offset, msg, used); - if (err <= 0) { + if (err < 0) { if (!copied) - copied = -EFAULT; + copied = err;
break; }