sunrpc: fix assignment of to_retries in svc_process_bc

Message ID 20240129-nfsd-fixes-v1-1-49a3a45bd750@kernel.org
State New
Headers
Series sunrpc: fix assignment of to_retries in svc_process_bc |

Commit Message

Jeff Layton Jan. 29, 2024, 4:43 p.m. UTC
  Alex reported seeing this:

    [   18.238266] ------------[ cut here ]------------
    [   18.239286] UBSAN: shift-out-of-bounds in net/sunrpc/xprt.c:660:14
    [   18.240699] shift exponent 60000 is too large for 64-bit type 'long unsigned int'
    [   18.242277] CPU: 1 PID: 290 Comm: NFSv4 callback Not tainted 6.8.0-rc1devtest5+ #5814
    [   18.243846] Hardware name: Red Hat KVM/RHEL-AV, BIOS 1.16.0-4.module+el8.9.0+19570+14a90618 04/01/2014
    [   18.245460] Call Trace:
    [   18.245855]  <TASK>
    [   18.246200]  dump_stack_lvl+0x93/0xb0
    [   18.246785]  dump_stack+0x10/0x20
    [   18.247308]  ubsan_epilogue+0x9/0x40
    [   18.247875]  __ubsan_handle_shift_out_of_bounds+0x110/0x170
    [   18.248727]  ? ktime_get+0x130/0x2a0
    [   18.249317]  xprt_calc_majortimeo.isra.13.cold.45+0x12/0x23
    [   18.250184]  xprt_init_majortimeo.isra.27+0x9c/0x150
    [   18.251062]  xprt_init_bc_request+0xc1/0xd0
    [   18.251728]  rpc_run_bc_task+0xd3/0x1b0
    [   18.252328]  ? __pfx_rpc_run_bc_task+0x10/0x10
    [   18.253045]  ? __this_cpu_preempt_check+0x13/0x20
    [   18.253780]  ? xdr_inline_decode+0x5b/0x260
    [   18.254447]  svc_process_bc+0x3b2/0x4d0
    [   18.255069]  ? __pfx_svc_process_bc+0x10/0x10
    [   18.255755]  ? __lwq_dequeue+0x5c/0xe0
    [   18.256350]  ? __kasan_check_read+0x11/0x20
    [   18.257002]  ? svc_thread_should_sleep+0x15d/0x190
    [   18.257754]  ? svc_recv+0x918/0x13b0
    [   18.258321]  svc_recv+0xa7e/0x13b0
    [   18.258892]  nfs4_callback_svc+0x53/0xb0
    [   18.259508]  ? __pfx_nfs4_callback_svc+0x10/0x10
    [   18.260227]  kthread+0x1c2/0x210
    [   18.260744]  ? kthread+0x103/0x210
    [   18.261278]  ? __pfx_kthread+0x10/0x10
    [   18.261872]  ret_from_fork+0x3a/0x50
    [   18.262433]  ? __pfx_kthread+0x10/0x10
    [   18.263024]  ret_from_fork_asm+0x1b/0x30
    [   18.263684]  </TASK>
    [   18.264348] ---[ end trace ]---

to_initval can be very large and cause a shift overflow later. Ensure we
copy the correct value into to_retries.

Cc: Benjamin Coddington <bcodding@redhat.com>
Reported-by: Alexander Aring <aahringo@redhat.com>
Fixes: 57331a59ac0d NFSv4.1: Use the nfs_client's rpc timeouts for backchannel
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 net/sunrpc/svc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


---
base-commit: 41bccc98fb7931d63d03f326a746ac4d429c1dd3
change-id: 20240129-nfsd-fixes-0d95718a0bca

Best regards,
  

Comments

Benjamin Coddington Jan. 29, 2024, 4:52 p.m. UTC | #1
On 29 Jan 2024, at 11:43, Jeff Layton wrote:

> Alex reported seeing this:
>
>     [   18.238266] ------------[ cut here ]------------
> ...

This one got fixed already, just waiting for a maintainer to send it along:

https://lore.kernel.org/linux-nfs/20240123053509.3592653-1-samasth.norway.ananda@oracle.com/

Ben
  
Chuck Lever Jan. 29, 2024, 4:55 p.m. UTC | #2
> On Jan 29, 2024, at 11:52 AM, Benjamin Coddington <bcodding@redhat.com> wrote:
> 
> On 29 Jan 2024, at 11:43, Jeff Layton wrote:
> 
>> Alex reported seeing this:
>> 
>>    [   18.238266] ------------[ cut here ]------------
>> ...
> 
> This one got fixed already, just waiting for a maintainer to send it along:
> 
> https://lore.kernel.org/linux-nfs/20240123053509.3592653-1-samasth.norway.ananda@oracle.com/

Ah. That touches net/sunrpc/svc.c, but it was sent to the client maintainers.

Do you want me to take it through the nfsd tree?


--
Chuck Lever
  
Jeff Layton Jan. 29, 2024, 4:59 p.m. UTC | #3
On Mon, 2024-01-29 at 11:52 -0500, Benjamin Coddington wrote:
> On 29 Jan 2024, at 11:43, Jeff Layton wrote:
> 
> > Alex reported seeing this:
> > 
> >     [   18.238266] ------------[ cut here ]------------
> > ...
> 
> This one got fixed already, just waiting for a maintainer to send it along:
> 
> https://lore.kernel.org/linux-nfs/20240123053509.3592653-1-samasth.norwayananda@oracle.com/
> 

Sorry for the noise. You can just drop mine.
  
Benjamin Coddington Jan. 29, 2024, 4:59 p.m. UTC | #4
On 29 Jan 2024, at 11:55, Chuck Lever III wrote:

>> On Jan 29, 2024, at 11:52 AM, Benjamin Coddington <bcodding@redhat.com> wrote:
>>
>> On 29 Jan 2024, at 11:43, Jeff Layton wrote:
>>
>>> Alex reported seeing this:
>>>
>>>    [   18.238266] ------------[ cut here ]------------
>>> ...
>>
>> This one got fixed already, just waiting for a maintainer to send it along:
>>
>> https://lore.kernel.org/linux-nfs/20240123053509.3592653-1-samasth.norway.ananda@oracle.com/
>
> Ah. That touches net/sunrpc/svc.c, but it was sent to the client maintainers.
>
> Do you want me to take it through the nfsd tree?

Oh yeah if you like, not sure how you want to sort it between the because
even though its in svc.c, its a client fix; this code is specific for the
client's backchannel.

Note the version on this thread misses the 2nd typo.

Ben
  
Chuck Lever Jan. 29, 2024, 5:02 p.m. UTC | #5
> On Jan 29, 2024, at 11:59 AM, Benjamin Coddington <bcodding@redhat.com> wrote:
> 
> On 29 Jan 2024, at 11:55, Chuck Lever III wrote:
> 
>>> On Jan 29, 2024, at 11:52 AM, Benjamin Coddington <bcodding@redhat.com> wrote:
>>> 
>>> On 29 Jan 2024, at 11:43, Jeff Layton wrote:
>>> 
>>>> Alex reported seeing this:
>>>> 
>>>>   [   18.238266] ------------[ cut here ]------------
>>>> ...
>>> 
>>> This one got fixed already, just waiting for a maintainer to send it along:
>>> 
>>> https://lore.kernel.org/linux-nfs/20240123053509.3592653-1-samasth.norway.ananda@oracle.com/
>> 
>> Ah. That touches net/sunrpc/svc.c, but it was sent to the client maintainers.
>> 
>> Do you want me to take it through the nfsd tree?
> 
> Oh yeah if you like, not sure how you want to sort it between the because
> even though its in svc.c, its a client fix; this code is specific for the
> client's backchannel.

I'll add to nfsd-fixes.


> Note the version on this thread misses the 2nd typo.

Ack.


--
Chuck Lever
  

Patch

diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
index f60c93e5a25d..d86bf5b051fa 100644
--- a/net/sunrpc/svc.c
+++ b/net/sunrpc/svc.c
@@ -1598,7 +1598,7 @@  void svc_process_bc(struct rpc_rqst *req, struct svc_rqst *rqstp)
 	/* Finally, send the reply synchronously */
 	if (rqstp->bc_to_initval > 0) {
 		timeout.to_initval = rqstp->bc_to_initval;
-		timeout.to_retries = rqstp->bc_to_initval;
+		timeout.to_retries = rqstp->bc_to_retries;
 	} else {
 		timeout.to_initval = req->rq_xprt->timeout->to_initval;
 		timeout.to_initval = req->rq_xprt->timeout->to_retries;