diff mbox series

sunrpc: fix assignment of to_retries in svc_process_bc

Message ID 20240129-nfsd-fixes-v1-1-49a3a45bd750@kernel.org (mailing list archive)
State Not Applicable
Delegated to: Netdev Maintainers
Headers show
Series sunrpc: fix assignment of to_retries in svc_process_bc | expand

Checks

Context Check Description
netdev/series_format warning Single patches do not need cover letters; Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1048 this patch: 1048
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 0 of 0 maintainers
netdev/build_clang success Errors and warnings before: 1065 this patch: 1065
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 1065 this patch: 1065
netdev/checkpatch warning WARNING: Please use correct Fixes: style 'Fixes: <12 chars of sha1> ("<title line>")' - ie: 'Fixes: 54c4cdace2b3 ("sunrpc: fix assignment of to_retries in svc_process_bc")'
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-01-31--00-00 (tests: 711)

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.norway.ananda@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
diff mbox series

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;