On Mon, Sep 09, 2024 at 11:33:04AM +0000, David Laight wrote:
> From: Scott Mayhew
> > Sent: 09 September 2024 12:24
> ...
> > > > > 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);
> > >
> > > Don't you also want an upper bound sanity check?
> > > (or is cn_len only 8 bit?)
> >
> > Yeah, actually it should probably be checking for namelen >
> > NFS4_OPAQUE_LIMIT.
Scott, can you send me a tested patch? I can squash that in.
> I suspect memdup_user() itself should have a third 'maxlen' argument.
> And probably one that is required to be a compile-time constant.
>
> Oh, and is dprintk() rate-limited?
No dprintk call site is rate-limited, and they are everywhere in
this code. Since they are off by default, rate-limiting is not a
concern. Also, journald will rate-limit any output from the kernel
to prevent flooding the system lock.
> Not that the message looks very helpful.
The specific message is not helpful; suggestions welcome. But I
prefer having a warning of some kind rather than a silent failure
mode.
--
Chuck Lever
