Received-SPF: pass (google.com: best guess record for domain of linux-nfs-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 23 Mar 2020 11:09:11 -0400
From:   "J. Bruce Fields" <bfields@redhat.com>
To:     Chuck Lever <chuck.lever@oracle.com>
Cc:     Vasily Averin <vvs@virtuozzo.com>,
        Jeff Layton <jlayton@kernel.org>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH] nfsd: memory corruption in nfsd4_lock()
Message-ID: <20200323150911.GA64741@pick.fieldses.org>
References: <db0980d0-8c99-940a-1748-04e679a366d1@virtuozzo.com>
 <5B820E18-763C-4562-9F50-56A0F1894406@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <5B820E18-763C-4562-9F50-56A0F1894406@oracle.com>
Sender: linux-nfs-owner@vger.kernel.org
Precedence: bulk

On Mon, Mar 23, 2020 at 09:50:34AM -0400, Chuck Lever wrote:
> 
> 
> > On Mar 23, 2020, at 3:55 AM, Vasily Averin <vvs@virtuozzo.com> wrote:
> > 
> > New struct nfsd4_blocked_lock allocated in find_or_allocate_block()
> > does not initialised nbl_list and nbl_lru.
> > If conflock allocation fails rollback can call list_del_init()
> > access uninitialized fields and corrupt memory.
> > 
> > Fixes: 76d348fadff5 ("nfsd: have nfsd4_lock use blocking locks for v4.1+ lock")
> > Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
> > ---
> > fs/nfsd/nfs4state.c | 32 +++++++++++++++-----------------
> > 1 file changed, 15 insertions(+), 17 deletions(-)
> > 
> > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > index 369e574c5092..176ef8d24fae 100644
> > --- a/fs/nfsd/nfs4state.c
> > +++ b/fs/nfsd/nfs4state.c
> > @@ -6524,6 +6524,13 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> > 		goto out;
> > 	}
> > 
> > +	conflock = locks_alloc_lock();
> > +	if (!conflock) {
> > +		dprintk("NFSD: %s: unable to allocate lock!\n", __func__);
> > +		status = nfserr_jukebox;
> > +		goto out;
> > +	}
> 
> Nit: What do people think about removing this dprintk() as part of the fix?

I don't think we want a dprintk every place we kmalloc.  All for
removing them.--b.