Re: UBSAN: shift-out-of-bounds in xprt_do_reserve

[Trond Myklebust <trondmy@hammerspace.com>]

Hi Randy,

On Wed, 2021-02-10 at 16:52 -0800, Randy Dunlap wrote:
> On 2/9/21 5:24 PM, syzbot wrote:
> > Hello,
> > 
> > syzbot found the following issue on:
> > 
> > HEAD commit:    dd86e7fa Merge tag 'pci-v5.11-fixes-2' of
> > git://git.kernel..
> > git tree:       upstream
> > console output:
> > https://syzkaller.appspot.com/x/log.txt?x=105930c4d00000
> > kernel config: 
> > https://syzkaller.appspot.com/x/.config?x=266a5362c89c8127
> > dashboard link:
> > https://syzkaller.appspot.com/bug?extid=f3a0fa110fd630ab56c8
> > compiler:       Debian clang version 11.0.1-2
> > syz repro:     
> > https://syzkaller.appspot.com/x/repro.syz?x=17ba3038d00000
> > C reproducer:  
> > https://syzkaller.appspot.com/x/repro.c?x=15cf0d64d00000
> > 
> > IMPORTANT: if you fix the issue, please add the following tag to
> > the commit:
> > Reported-by: syzbot+f3a0fa110fd630ab56c8@syzkaller.appspotmail.com
> 
> #syz dup: UBSAN: shift-out-of-bounds in xprt_calc_majortimeo
> 
> > ===================================================================
> > =============
> > UBSAN: shift-out-of-bounds in net/sunrpc/xprt.c:658:14
> > shift exponent 536870976 is too large for 64-bit type 'unsigned
> > long'
> > CPU: 1 PID: 8411 Comm: syz-executor902 Not tainted 5.11.0-rc6-
> > syzkaller #0
> > Hardware name: Google Google Compute Engine/Google Compute Engine,
> > BIOS Google 01/01/2011
> > Call Trace:
> >  __dump_stack lib/dump_stack.c:79 [inline]
> >  dump_stack+0x137/0x1be lib/dump_stack.c:120
> >  ubsan_epilogue lib/ubsan.c:148 [inline]
> >  __ubsan_handle_shift_out_of_bounds+0x432/0x4d0 lib/ubsan.c:395
> >  xprt_calc_majortimeo net/sunrpc/xprt.c:658 [inline]
> >  xprt_init_majortimeo net/sunrpc/xprt.c:686 [inline]
> 
> 

So, firstly, this is a case of 'doctor it hurts when I do this...' so
it isn't a critcal issue. It is a case where garbage mount options
produces garbage timeout values.
However, more importantly, it is a case where we could easily be
checking these values once at mount time instead of adding runtime
checks that can end up being called several times per RPC call.

Cheers
  Trond
-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com