linux-kernel - Re: [PATCH] nfsd: return -EINVAL when namelen is 0

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Ztcq9RB6SPhxbDl5@tissot.1015granger.net>
Date: Tue, 3 Sep 2024 11:27:49 -0400
From: Chuck Lever <chuck.lever@...cle.com>
To: Jeff Layton <jlayton@...nel.org>, Scott Mayhew <smayhew@...hat.com>
Cc: Li Lingfeng <lilingfeng3@...wei.com>, neilb@...e.de, okorniev@...hat.com,
        Dai.Ngo@...cle.com, tom@...pey.com, linux-nfs@...r.kernel.org,
        linux-kernel@...r.kernel.org, yukuai1@...weicloud.com,
        houtao1@...wei.com, yi.zhang@...wei.com, yangerkun@...wei.com,
        lilingfeng@...weicloud.com
Subject: Re: [PATCH] nfsd: return -EINVAL when namelen is 0

On Tue, Sep 03, 2024 at 07:23:18AM -0400, Jeff Layton wrote:
> On Tue, 2024-09-03 at 19:14 +0800, Li Lingfeng wrote:
> > When we have a corrupted main.sqlite in /var/lib/nfs/nfsdcld/, it may
> > result in namelen being 0, which will cause memdup_user() to return
> > ZERO_SIZE_PTR.
> > When we access the name.data that has been assigned the value of
> > ZERO_SIZE_PTR in nfs4_client_to_reclaim(), null pointer dereference is
> > triggered.
> > 
> > [ T1205] ==================================================================
> > [ T1205] BUG: KASAN: null-ptr-deref in nfs4_client_to_reclaim+0xe9/0x260
> > [ T1205] Read of size 1 at addr 0000000000000010 by task nfsdcld/1205
> > [ T1205]
> > [ T1205] CPU: 11 PID: 1205 Comm: nfsdcld Not tainted 5.10.0-00003-g2c1423731b8d #406
> > [ T1205] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS ?-20190727_073836-buildvm-ppc64le-16.ppc.fedoraproject.org-3.fc31 04/01/2014
> > [ T1205] Call Trace:
> > [ T1205]  dump_stack+0x9a/0xd0
> > [ T1205]  ? nfs4_client_to_reclaim+0xe9/0x260
> > [ T1205]  __kasan_report.cold+0x34/0x84
> > [ T1205]  ? nfs4_client_to_reclaim+0xe9/0x260
> > [ T1205]  kasan_report+0x3a/0x50
> > [ T1205]  nfs4_client_to_reclaim+0xe9/0x260
> > [ T1205]  ? nfsd4_release_lockowner+0x410/0x410
> > [ T1205]  cld_pipe_downcall+0x5ca/0x760
> > [ T1205]  ? nfsd4_cld_tracking_exit+0x1d0/0x1d0
> > [ T1205]  ? down_write_killable_nested+0x170/0x170
> > [ T1205]  ? avc_policy_seqno+0x28/0x40
> > [ T1205]  ? selinux_file_permission+0x1b4/0x1e0
> > [ T1205]  rpc_pipe_write+0x84/0xb0
> > [ T1205]  vfs_write+0x143/0x520
> > [ T1205]  ksys_write+0xc9/0x170
> > [ T1205]  ? __ia32_sys_read+0x50/0x50
> > [ T1205]  ? ktime_get_coarse_real_ts64+0xfe/0x110
> > [ T1205]  ? ktime_get_coarse_real_ts64+0xa2/0x110
> > [ T1205]  do_syscall_64+0x33/0x40
> > [ T1205]  entry_SYSCALL_64_after_hwframe+0x67/0xd1
> > [ T1205] RIP: 0033:0x7fdbdb761bc7
> > [ T1205] Code: 0f 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b7 0f 1f 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 514
> > [ T1205] RSP: 002b:00007fff8c4b7248 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
> > [ T1205] RAX: ffffffffffffffda RBX: 000000000000042b RCX: 00007fdbdb761bc7
> > [ T1205] RDX: 000000000000042b RSI: 00007fff8c4b75f0 RDI: 0000000000000008
> > [ T1205] RBP: 00007fdbdb761bb0 R08: 0000000000000000 R09: 0000000000000001
> > [ T1205] R10: 0000000000000000 R11: 0000000000000246 R12: 000000000000042b
> > [ T1205] R13: 0000000000000008 R14: 00007fff8c4b75f0 R15: 0000000000000000
> > [ T1205] ==================================================================
> > 
> > Fix it by checking namelen.
> > 
> > Signed-off-by: Li Lingfeng <lilingfeng3@...wei.com>
> > ---
> >  fs/nfsd/nfs4recover.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
> > index 67d8673a9391..69a3a84e159e 100644
> > --- a/fs/nfsd/nfs4recover.c
> > +++ b/fs/nfsd/nfs4recover.c
> > @@ -809,6 +809,10 @@ __cld_pipe_inprogress_downcall(const struct cld_msg_v2 __user *cmsg,
> >  			ci = &cmsg->cm_u.cm_clntinfo;
> >  			if (get_user(namelen, &ci->cc_name.cn_len))
> >  				return -EFAULT;
> > +			if (!namelen) {
> > +				dprintk("%s: namelen should not be zero", __func__);
> > +				return -EINVAL;
> > +			}
> >  			name.data = memdup_user(&ci->cc_name.cn_id, namelen);
> >  			if (IS_ERR(name.data))
> >  				return PTR_ERR(name.data);
> > @@ -831,6 +835,10 @@ __cld_pipe_inprogress_downcall(const struct cld_msg_v2 __user *cmsg,
> >  			cnm = &cmsg->cm_u.cm_name;
> >  			if (get_user(namelen, &cnm->cn_len))
> >  				return -EFAULT;
> > +			if (!namelen) {
> > +				dprintk("%s: namelen should not be zero", __func__);
> > +				return -EINVAL;
> > +			}
> >  			name.data = memdup_user(&cnm->cn_id, namelen);
> >  			if (IS_ERR(name.data))
> >  				return PTR_ERR(name.data);
> 
> This error will come back to nfsdcld in its downcall write(). -EINVAL
> is a bit generic. It might be better to go with a more distinct error,
> but I don't think it matters too much.
> 
> Reviewed-by: Jeff Layton <jlayton@...nel.org>

Wondering if

   Fixes: 74725959c33c ("nfsd: un-deprecate nfsdcld")

would be appropriate.


-- 
Chuck Lever