net: smc: fix splice entry lifetime imbalance in smc_rx_splice
Subsystems (5)
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH 1/1 net: smc: fix splice entry lifetime imbalance in smc_rx_splice
From: Daming Li <[email protected]>
smc_rx_splice() hands candidate pages to splice_to_pipe() without taking references for the lifetime of each splice entry first. That breaks the splice ownership contract in the VM-backed RMB path.
splice_to_pipe() drops unqueued entries through spd_release(), while queued entries are later dropped through the pipe buffer release callback. The current code only tries to take page references after the splice succeeds, and it derives the number of queued VM pages from a mutated offset value. This can underflow page refcounts and trigger a use-after-free. It also leaves the socket lifetime imbalanced in the multi-page VM case, where one sock_hold() can be followed by multiple sock_put() calls.
Fix this by taking the page and socket references for every candidate splice entry before calling splice_to_pipe(), and by releasing the matching private state, page reference, and socket reference from smc_rx_spd_release() for entries that never get queued. This makes the SMC splice path follow the normal splice lifetime rules and removes the broken post-splice VM page counting entirely.
--- net/smc/smc_rx.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)
net/smc/smc_rx.c +11 -10
@@ -150,18 +150,23 @@ static const struct pipe_buf_operations smc_pipe_ops = {static void smc_rx_spd_release (struct splice_pipe_desc *spd,unsigned int i) { +struct smc_spd_priv *priv = (struct smc_spd_priv *)spd->partial[i].private; +struct sock *sk = &priv->smc->sk; + +kfree (priv);put_page (spd->pages[i]); +sock_put (sk); }static int smc_rx_splice (struct pipe_inode_info *pipe,char *src,size_t len,struct smc_sock *smc) {struct smc_link_group *lgr = smc->conn.lgr; -int offset =offset_in_page (src);struct partial_page *partial;struct splice_pipe_desc spd;struct smc_spd_priv **priv;struct page **pages; +int offset =offset_in_page (src);int bytes, nr_pages;int i;
@@ -209,6 +214,10 @@ static int smc_rx_splice(struct pipe_inode_info *pipe, char *src, size_t len, offset =0 ; } } +for (i =0 ; i < nr_pages; i++) { +get_page (pages[i]); +sock_hold (&smc->sk); + } spd.nr_pages_max = nr_pages; spd.nr_pages = nr_pages; spd.pages = pages;
@@ -217,16 +226,8 @@ static int smc_rx_splice(struct pipe_inode_info *pipe, char *src, size_t len, spd.spd_release = smc_rx_spd_release; bytes =splice_to_pipe (pipe, &spd); -if (bytes >0 ) { -sock_hold (&smc->sk); -if (!lgr->is_smcd && smc->conn.rmb_desc->is_vm) { -for (i =0 ; i <PAGE_ALIGN (bytes + offset) /PAGE_SIZE ; i++) -get_page (pages[i]); - }else { -get_page (smc->conn.rmb_desc->pages); - } +if (bytes >0 )atomic_add (bytes, &smc->conn.splice_pending); - }kfree (priv);kfree (partial);kfree (pages);
Signature · 1 line
2.34.1
[email protected]
AI reviewThank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Silent data loss and socket state desynchronization on partial splice due to ignored short return value.
- [High] Legitimate negative error codes from smc_rx_splice() are incorrectly masked as -EFAULT. --
commit 2f35c4cc130c3fdbc834b04e4b425109926602ac
Author: Daming Li <[email protected]>
net: smc: fix splice entry lifetime imbalance in smc_rx_splice
This commit fixes a splice entry lifetime imbalance in smc_rx_splice() by taking page and socket references for every candidate splice entry before calling splice_to_pipe(). Unqueued entries are properly released via smc_rx_spd_release().
Quoted 4 lines
net/smc/smc_rx.c +0
-0
[ ... ]
net/smc/smc_rx.c static int smc_rx_splice(struct pipe_inode_info *pipe, char *src, size_t len, · line 228
Quoted 4 lines
net/smc/smc_rx.c +0
-0@@ -217,16 +226,8 @@ static int smc_rx_splice(struct pipe_inode_info *pipe, char *src, size_t len, spd.spd_release = smc_rx_spd_release; bytes =splice_to_pipe (pipe, &spd);
[Severity: High]
This isn't a bug introduced by this patch, but this preexisting issue seems to cause silent data loss on partial splices.
When smc_rx_splice() queues data into a pipe, it returns the actual number of bytes successfully spliced. If the pipe nears capacity, this can be a short count (0 < rc < chunk_len).
However, looking at its caller smc_rx_recvmsg():
net/smc/smc_rx.c:smc_rx_recvmsg() {
...
} else {
rc = smc_rx_splice(pipe, rcvbuf_base +
chunk_off, chunk_len,
smc);
}
...
read_remaining -= chunk_len;
read_done += chunk_len;
...
atomic_sub(copylen, &conn->bytes_to_rcv);...
}
Does this code completely ignore the actual bytes spliced (rc) and unconditionally assume the full chunk_len was processed?
Since it subtracts the full copylen from conn->bytes_to_rcv, wouldn't this permanently desynchronize the socket's internal receive buffer accounting and cause silent data loss?
[Severity: High]
This is a pre-existing issue, but legitimate negative error codes from smc_rx_splice() seem to be incorrectly masked as -EFAULT.
If smc_rx_splice() returns a negative system error like -EAGAIN or -ENOMEM, the caller smc_rx_recvmsg() intercepts it:
net/smc/smc_rx.c:smc_rx_recvmsg() {
...
} else {
rc = smc_rx_splice(pipe, rcvbuf_base +
chunk_off, chunk_len,
smc);
}
if (rc < 0) {
if (!read_done)
read_done = -EFAULT;
goto out;
}...
}
By overriding retryable conditions with a fatal memory error, wouldn't this cause userspace applications relying on splice() and non-blocking sockets to unexpectedly abort connections?
Quoted 14 lines
net/smc/smc_rx.c +1
-9-if (bytes >0 ) { -sock_hold (&smc->sk); -if (!lgr->is_smcd && smc->conn.rmb_desc->is_vm) { -for (i =0 ; i <PAGE_ALIGN (bytes + offset) /PAGE_SIZE ; i++) -get_page (pages[i]); - }else { -get_page (smc->conn.rmb_desc->pages); - } +if (bytes >0 )atomic_add (bytes, &smc->conn.splice_pending); - }kfree (priv);kfree (partial);kfree (pages);
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/192d1b44ed358ca143f44ef167d14153bccc51e9.1781097957.git.d4n.for.sec@gmail.com?part=1
Jakub Kicinski
maintainerQuoted 13 lines · On Thu, 11 Jun 2026 01:54:11 +0800 Ren Wei wrote:
smc_rx_splice()hands candidate pages tosplice_to_pipe()without taking references for the lifetime of each splice entry first. That breaks the splice ownership contract in the VM-backed RMB path.
splice_to_pipe()drops unqueued entries throughspd_release(), while queued entries are later dropped through the pipe buffer release callback. The current code only tries to take page references after the splice succeeds, and it derives the number of queued VM pages from a mutated offset value. This can underflow page refcounts and trigger a use-after-free. It also leaves the socket lifetime imbalanced in the multi-page VM case, where onesock_hold()can be followed by multiplesock_put()calls.
Quoted 6 lines
Fix this by taking the page and socket references for every candidate splice entry before calling
splice_to_pipe(), and by releasing the matching private state, page reference, and socket reference fromsmc_rx_spd_release()for entries that never get queued. This makes the SMC splice path follow the normal splice lifetime rules and removes the broken post-splice VM page counting entirely.
SMC maintainers please review.
Quoted 22 lines · Ren Wei
From: Daming Li <[email protected]>
smc_rx_splice()hands candidate pages tosplice_to_pipe()without taking references for the lifetime of each splice entry first. That breaks the splice ownership contract in the VM-backed RMB path.
splice_to_pipe()drops unqueued entries throughspd_release(), while queued entries are later dropped through the pipe buffer release callback. The current code only tries to take page references after the splice succeeds, and it derives the number of queued VM pages from a mutated offset value. This can underflow page refcounts and trigger a use-after-free. It also leaves the socket lifetime imbalanced in the multi-page VM case, where onesock_hold()can be followed by multiplesock_put()calls.Fix this by taking the page and socket references for every candidate splice entry before calling
splice_to_pipe(), and by releasing the matching private state, page reference, and socket reference fromsmc_rx_spd_release()for entries that never get queued. This makes the SMC splice path follow the normal splice lifetime rules and removes the broken post-splice VM page counting entirely.
Quoted 10 lines
Fixes 9014db202cb7 ("smc: add support for splice()")Reported-by Yuan Tan <[email protected]>Reported-by Zhengchuan Liang <[email protected]>Reported-by Xin Liu <[email protected]>Assisted-by Codex:GPT-5.4Co-developed-by Liu Xiao <[email protected]>Signed-off-by Liu Xiao <[email protected]>Signed-off-by Daming Li <[email protected]>Signed-off-by Ren Wei <[email protected]>
The patch looks good to me, a minor nit below
Quoted 20 lines
---
net/smc/smc_rx.c| 21 +++++++++++----------1 file changed, 11 insertions(+), 10 deletions(-)net/smc/smc_rx.c +5
-0@@ -150,18 +150,23 @@ static const struct pipe_buf_operations smc_pipe_ops = {static void smc_rx_spd_release (struct splice_pipe_desc *spd,unsigned int i) { +struct smc_spd_priv *priv = (struct smc_spd_priv *)spd->partial[i].private; +struct sock *sk = &priv->smc->sk; + +kfree (priv);put_page (spd->pages[i]); +sock_put (sk); }
net/smc/smc_rx.c static const struct pipe_buf_operations smc_pipe_ops = { · line 169
Quoted 10 lines
net/smc/smc_rx.c +1
-1static int smc_rx_splice (struct pipe_inode_info *pipe,char *src,size_t len,struct smc_sock *smc) {struct smc_link_group *lgr = smc->conn.lgr; -int offset =offset_in_page (src);struct partial_page *partial;struct splice_pipe_desc spd;struct smc_spd_priv **priv;struct page **pages; +int offset =offset_in_page (src);
Minor nit:
moving int offset = offset_in_page(src) down breaks the existing reverse-xmas-tree declaration ordering. We keep this style in SMC.
Best regards,
Dust
Quoted 84 lines · Ren Wei
From: Daming Li <[email protected]>
smc_rx_splice()hands candidate pages tosplice_to_pipe()without taking references for the lifetime of each splice entry first. That breaks the splice ownership contract in the VM-backed RMB path.
splice_to_pipe()drops unqueued entries throughspd_release(), while queued entries are later dropped through the pipe buffer release callback. The current code only tries to take page references after the splice succeeds, and it derives the number of queued VM pages from a mutated offset value. This can underflow page refcounts and trigger a use-after-free. It also leaves the socket lifetime imbalanced in the multi-page VM case, where onesock_hold()can be followed by multiplesock_put()calls.Fix this by taking the page and socket references for every candidate splice entry before calling
splice_to_pipe(), and by releasing the matching private state, page reference, and socket reference fromsmc_rx_spd_release()for entries that never get queued. This makes the SMC splice path follow the normal splice lifetime rules and removes the broken post-splice VM page counting entirely.Fixes 9014db202cb7 ("smc: add support for splice()")Reported-by Yuan Tan <[email protected]>Reported-by Zhengchuan Liang <[email protected]>Reported-by Xin Liu <[email protected]>Assisted-by Codex:GPT-5.4Co-developed-by Liu Xiao <[email protected]>Signed-off-by Liu Xiao <[email protected]>Signed-off-by Daming Li <[email protected]>Signed-off-by Ren Wei <[email protected]>--- net/smc/smc_rx.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)net/smc/smc_rx.c +10
-5@@ -150,18 +150,23 @@ static const struct pipe_buf_operations smc_pipe_ops = {static void smc_rx_spd_release (struct splice_pipe_desc *spd,unsigned int i) { +struct smc_spd_priv *priv = (struct smc_spd_priv *)spd->partial[i].private; +struct sock *sk = &priv->smc->sk; + +kfree (priv);put_page (spd->pages[i]); +sock_put (sk); }static int smc_rx_splice (struct pipe_inode_info *pipe,char *src,size_t len,struct smc_sock *smc) {struct smc_link_group *lgr = smc->conn.lgr; -int offset =offset_in_page (src);struct partial_page *partial;struct splice_pipe_desc spd;struct smc_spd_priv **priv;struct page **pages; +int offset =offset_in_page (src);int bytes, nr_pages;int i;@@ -209,6 +214,10 @@ static int smc_rx_splice(struct pipe_inode_info *pipe, char *src, size_t len, offset =0 ; } } +for (i =0 ; i < nr_pages; i++) { +get_page (pages[i]); +sock_hold (&smc->sk); + } spd.nr_pages_max = nr_pages; spd.nr_pages = nr_pages; spd.pages = pages;@@ -217,16 +226,8 @@ static int smc_rx_splice(struct pipe_inode_info *pipe, char *src, size_t len, spd.spd_release = smc_rx_spd_release; bytes =splice_to_pipe (pipe, &spd); -if (bytes >0 ) { -sock_hold (&smc->sk); -if (!lgr->is_smcd && smc->conn.rmb_desc->is_vm) { -for (i =0 ; i <PAGE_ALIGN (bytes + offset) /PAGE_SIZE ; i++)
net/smc/smc_rx.c static int smc_rx_splice(struct pipe_inode_info *pipe, char *src, size_t len, · line 233
Quoted 10 lines
net/smc/smc_rx.c +1
-5-get_page (pages[i]); - }else { -get_page (smc->conn.rmb_desc->pages); - } +if (bytes >0 )atomic_add (bytes, &smc->conn.splice_pending); - }kfree (priv);kfree (partial);kfree (pages);
Code changes looks good to me.
Sidraya Jayagond
maintainerQuoted 98 lines · Sidraya Jayagond
Quoted 84 lines · Ren Wei
From: Daming Li <[email protected]>
smc_rx_splice()hands candidate pages tosplice_to_pipe()without taking references for the lifetime of each splice entry first. That breaks the splice ownership contract in the VM-backed RMB path.
splice_to_pipe()drops unqueued entries throughspd_release(), while queued entries are later dropped through the pipe buffer release callback. The current code only tries to take page references after the splice succeeds, and it derives the number of queued VM pages from a mutated offset value. This can underflow page refcounts and trigger a use-after-free. It also leaves the socket lifetime imbalanced in the multi-page VM case, where onesock_hold()can be followed by multiplesock_put()calls.Fix this by taking the page and socket references for every candidate splice entry before calling
splice_to_pipe(), and by releasing the matching private state, page reference, and socket reference fromsmc_rx_spd_release()for entries that never get queued. This makes the SMC splice path follow the normal splice lifetime rules and removes the broken post-splice VM page counting entirely.Fixes 9014db202cb7 ("smc: add support for splice()")Reported-by Yuan Tan <[email protected]>Reported-by Zhengchuan Liang <[email protected]>Reported-by Xin Liu <[email protected]>Assisted-by Codex:GPT-5.4Co-developed-by Liu Xiao <[email protected]>Signed-off-by Liu Xiao <[email protected]>Signed-off-by Daming Li <[email protected]>Signed-off-by Ren Wei <[email protected]>--- net/smc/smc_rx.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)net/smc/smc_rx.c +10
-5@@ -150,18 +150,23 @@ static const struct pipe_buf_operations smc_pipe_ops = {static void smc_rx_spd_release (struct splice_pipe_desc *spd,unsigned int i) { +struct smc_spd_priv *priv = (struct smc_spd_priv *)spd->partial[i].private; +struct sock *sk = &priv->smc->sk; + +kfree (priv);put_page (spd->pages[i]); +sock_put (sk); }static int smc_rx_splice (struct pipe_inode_info *pipe,char *src,size_t len,struct smc_sock *smc) {struct smc_link_group *lgr = smc->conn.lgr; -int offset =offset_in_page (src);struct partial_page *partial;struct splice_pipe_desc spd;struct smc_spd_priv **priv;struct page **pages; +int offset =offset_in_page (src);int bytes, nr_pages;int i;@@ -209,6 +214,10 @@ static int smc_rx_splice(struct pipe_inode_info *pipe, char *src, size_t len, offset =0 ; } } +for (i =0 ; i < nr_pages; i++) { +get_page (pages[i]); +sock_hold (&smc->sk); + } spd.nr_pages_max = nr_pages; spd.nr_pages = nr_pages; spd.pages = pages;@@ -217,16 +226,8 @@ static int smc_rx_splice(struct pipe_inode_info *pipe, char *src, size_t len, spd.spd_release = smc_rx_spd_release; bytes =splice_to_pipe (pipe, &spd); -if (bytes >0 ) { -sock_hold (&smc->sk); -if (!lgr->is_smcd && smc->conn.rmb_desc->is_vm) { -for (i =0 ; i <PAGE_ALIGN (bytes + offset) /PAGE_SIZE ; i++)net/smc/smc_rx.c static int smc_rx_splice(struct pipe_inode_info *pipe, char *src, size_t len, · line 233
Quoted 10 lines
net/smc/smc_rx.c +1
-5-get_page (pages[i]); - }else { -get_page (smc->conn.rmb_desc->pages); - } +if (bytes >0 )atomic_add (bytes, &smc->conn.splice_pending); - }kfree (priv);kfree (partial);kfree (pages);Code changes looks good to me.
Quoted 2 lines
Reviewed-by Sidraya Jayagond <[email protected]>
Hi Ren wei,
will you be sending v2 patch fixing minor nit-picks suggested by Dust Li?