Re: [PATCH net 1/1] net: smc: fix splice entry lifetime imbalance in smc_rx_splice

[Sidraya Jayagond <sidraya@linux.ibm.com>]

On 16/06/26 7:57 pm, Sidraya Jayagond wrote:
> 
> 
> On 10/06/26 11:24 pm, Ren Wei wrote:
>> From: Daming Li <d4n.for.sec@gmail.com>
>>
>> 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.
>>
>> Fixes: 9014db202cb7 ("smc: add support for splice()")
>> Cc: stable@vger.kernel.org
>> Reported-by: Yuan Tan <yuantan098@gmail.com>
>> Reported-by: Zhengchuan Liang <zcliangcn@gmail.com>
>> Reported-by: Xin Liu <bird@lzu.edu.cn>
>> Assisted-by: Codex:GPT-5.4
>> Co-developed-by: Liu Xiao <lx24@stu.ynu.edu.cn>
>> Signed-off-by: Liu Xiao <lx24@stu.ynu.edu.cn>
>> Signed-off-by: Daming Li <d4n.for.sec@gmail.com>
>> Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
>> ---
>>  net/smc/smc_rx.c | 21 +++++++++++----------
>>  1 file changed, 11 insertions(+), 10 deletions(-)
>>
>> diff --git a/net/smc/smc_rx.c b/net/smc/smc_rx.c
>> index c1d9b923938d..88aee0d93597 100644
>> --- a/net/smc/smc_rx.c
>> +++ b/net/smc/smc_rx.c
>> @@ -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);
> Code changes looks good to me.
> Reviewed-by: Sidraya Jayagond <sidraya@linux.ibm.com>
> 
Hi Ren wei,
will you be sending v2 patch fixing minor nit-picks suggested by Dust Li?