linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] nfsd: memory corruption in nfsd4_lock()
@ 2020-03-23  7:55 Vasily Averin
  2020-03-23 12:18 ` Jeff Layton
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Vasily Averin @ 2020-03-23  7:55 UTC (permalink / raw)
  To: J. Bruce Fields, Chuck Lever, Jeff Layton; +Cc: linux-nfs

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;
+	}
+
 	nbl = find_or_allocate_block(lock_sop, &fp->fi_fhandle, nn);
 	if (!nbl) {
 		dprintk("NFSD: %s: unable to allocate block!\n", __func__);
@@ -6542,13 +6549,6 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	file_lock->fl_end = last_byte_offset(lock->lk_offset, lock->lk_length);
 	nfs4_transform_lock_offset(file_lock);
 
-	conflock = locks_alloc_lock();
-	if (!conflock) {
-		dprintk("NFSD: %s: unable to allocate lock!\n", __func__);
-		status = nfserr_jukebox;
-		goto out;
-	}
-
 	if (fl_flags & FL_SLEEP) {
 		nbl->nbl_time = jiffies;
 		spin_lock(&nn->blocked_locks_lock);
@@ -6581,17 +6581,15 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		status = nfserrno(err);
 		break;
 	}
-out:
-	if (nbl) {
-		/* dequeue it if we queued it before */
-		if (fl_flags & FL_SLEEP) {
-			spin_lock(&nn->blocked_locks_lock);
-			list_del_init(&nbl->nbl_list);
-			list_del_init(&nbl->nbl_lru);
-			spin_unlock(&nn->blocked_locks_lock);
-		}
-		free_blocked_lock(nbl);
+	/* dequeue it if we queued it before */
+	if (fl_flags & FL_SLEEP) {
+		spin_lock(&nn->blocked_locks_lock);
+		list_del_init(&nbl->nbl_list);
+		list_del_init(&nbl->nbl_lru);
+		spin_unlock(&nn->blocked_locks_lock);
 	}
+	free_blocked_lock(nbl);
+out:
 	if (nf)
 		nfsd_file_put(nf);
 	if (lock_stp) {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] nfsd: memory corruption in nfsd4_lock()
  2020-03-23  7:55 [PATCH] nfsd: memory corruption in nfsd4_lock() Vasily Averin
@ 2020-03-23 12:18 ` Jeff Layton
  2020-03-23 22:12   ` Vasily Averin
  2020-03-23 13:50 ` Chuck Lever
  2020-03-26 15:55 ` Chuck Lever
  2 siblings, 1 reply; 9+ messages in thread
From: Jeff Layton @ 2020-03-23 12:18 UTC (permalink / raw)
  To: Vasily Averin, J. Bruce Fields, Chuck Lever; +Cc: linux-nfs

On Mon, 2020-03-23 at 10:55 +0300, Vasily Averin 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;
> +	}
> +
>  	nbl = find_or_allocate_block(lock_sop, &fp->fi_fhandle, nn);
>  	if (!nbl) {
>  		dprintk("NFSD: %s: unable to allocate block!\n", __func__);
> @@ -6542,13 +6549,6 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  	file_lock->fl_end = last_byte_offset(lock->lk_offset, lock->lk_length);
>  	nfs4_transform_lock_offset(file_lock);
>  
> -	conflock = locks_alloc_lock();
> -	if (!conflock) {
> -		dprintk("NFSD: %s: unable to allocate lock!\n", __func__);
> -		status = nfserr_jukebox;
> -		goto out;
> -	}
> -
>  	if (fl_flags & FL_SLEEP) {
>  		nbl->nbl_time = jiffies;
>  		spin_lock(&nn->blocked_locks_lock);
> @@ -6581,17 +6581,15 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  		status = nfserrno(err);
>  		break;
>  	}
> -out:
> -	if (nbl) {
> -		/* dequeue it if we queued it before */
> -		if (fl_flags & FL_SLEEP) {
> -			spin_lock(&nn->blocked_locks_lock);
> -			list_del_init(&nbl->nbl_list);
> -			list_del_init(&nbl->nbl_lru);
> -			spin_unlock(&nn->blocked_locks_lock);
> -		}
> -		free_blocked_lock(nbl);
> +	/* dequeue it if we queued it before */
> +	if (fl_flags & FL_SLEEP) {
> +		spin_lock(&nn->blocked_locks_lock);
> +		list_del_init(&nbl->nbl_list);
> +		list_del_init(&nbl->nbl_lru);
> +		spin_unlock(&nn->blocked_locks_lock);
>  	}
> +	free_blocked_lock(nbl);
> +out:
>  	if (nf)
>  		nfsd_file_put(nf);
>  	if (lock_stp) {

Good catch! Is there any reason not to just fix this by initializing the
list_heads in find_or_allocate_block? That seems like it'd be a simpler
fix.

-- 
Jeff Layton <jlayton@kernel.org>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] nfsd: memory corruption in nfsd4_lock()
  2020-03-23  7:55 [PATCH] nfsd: memory corruption in nfsd4_lock() Vasily Averin
  2020-03-23 12:18 ` Jeff Layton
@ 2020-03-23 13:50 ` Chuck Lever
  2020-03-23 15:09   ` J. Bruce Fields
  2020-03-26 15:55 ` Chuck Lever
  2 siblings, 1 reply; 9+ messages in thread
From: Chuck Lever @ 2020-03-23 13:50 UTC (permalink / raw)
  To: Vasily Averin; +Cc: Bruce Fields, Jeff Layton, Linux NFS Mailing List



> 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?


> 	nbl = find_or_allocate_block(lock_sop, &fp->fi_fhandle, nn);
> 	if (!nbl) {
> 		dprintk("NFSD: %s: unable to allocate block!\n", __func__);
> @@ -6542,13 +6549,6 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> 	file_lock->fl_end = last_byte_offset(lock->lk_offset, lock->lk_length);
> 	nfs4_transform_lock_offset(file_lock);
> 
> -	conflock = locks_alloc_lock();
> -	if (!conflock) {
> -		dprintk("NFSD: %s: unable to allocate lock!\n", __func__);
> -		status = nfserr_jukebox;
> -		goto out;
> -	}
> -
> 	if (fl_flags & FL_SLEEP) {
> 		nbl->nbl_time = jiffies;
> 		spin_lock(&nn->blocked_locks_lock);
> @@ -6581,17 +6581,15 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> 		status = nfserrno(err);
> 		break;
> 	}
> -out:
> -	if (nbl) {
> -		/* dequeue it if we queued it before */
> -		if (fl_flags & FL_SLEEP) {
> -			spin_lock(&nn->blocked_locks_lock);
> -			list_del_init(&nbl->nbl_list);
> -			list_del_init(&nbl->nbl_lru);
> -			spin_unlock(&nn->blocked_locks_lock);
> -		}
> -		free_blocked_lock(nbl);
> +	/* dequeue it if we queued it before */
> +	if (fl_flags & FL_SLEEP) {
> +		spin_lock(&nn->blocked_locks_lock);
> +		list_del_init(&nbl->nbl_list);
> +		list_del_init(&nbl->nbl_lru);
> +		spin_unlock(&nn->blocked_locks_lock);
> 	}
> +	free_blocked_lock(nbl);
> +out:
> 	if (nf)
> 		nfsd_file_put(nf);
> 	if (lock_stp) {
> -- 
> 2.17.1
> 

--
Chuck Lever




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] nfsd: memory corruption in nfsd4_lock()
  2020-03-23 13:50 ` Chuck Lever
@ 2020-03-23 15:09   ` J. Bruce Fields
  0 siblings, 0 replies; 9+ messages in thread
From: J. Bruce Fields @ 2020-03-23 15:09 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Vasily Averin, Jeff Layton, Linux NFS Mailing List

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.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] nfsd: memory corruption in nfsd4_lock()
  2020-03-23 12:18 ` Jeff Layton
@ 2020-03-23 22:12   ` Vasily Averin
  0 siblings, 0 replies; 9+ messages in thread
From: Vasily Averin @ 2020-03-23 22:12 UTC (permalink / raw)
  To: Jeff Layton, J. Bruce Fields, Chuck Lever; +Cc: linux-nfs

On 3/23/20 3:18 PM, Jeff Layton wrote:
> On Mon, 2020-03-23 at 10:55 +0300, Vasily Averin 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>
> 
> Good catch! Is there any reason not to just fix this by initializing the
> list_heads in find_or_allocate_block? That seems like it'd be a simpler
> fix.
> 

Rollback in nfsd4_lock() is not optimal, I've tried to improve it too,
However I agree such improvement is not a simplest fix
and it anyway does not make whole rollback perfect.

I think it's better to re-send small fix for the found problem,
and prepare separate patches for rollback improvements,

Thank you,
	Vasily Averin

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] nfsd: memory corruption in nfsd4_lock()
  2020-03-23  7:55 [PATCH] nfsd: memory corruption in nfsd4_lock() Vasily Averin
  2020-03-23 12:18 ` Jeff Layton
  2020-03-23 13:50 ` Chuck Lever
@ 2020-03-26 15:55 ` Chuck Lever
  2020-03-27  4:50   ` [PATCH v2] " Vasily Averin
  2 siblings, 1 reply; 9+ messages in thread
From: Chuck Lever @ 2020-03-26 15:55 UTC (permalink / raw)
  To: Vasily Averin, Jeff Layton; +Cc: Bruce Fields, Linux NFS Mailing List

Howdy-

> 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>

I'm not certain where we stand with this one. Jeff, are you OK
with me taking this for v5.7, or is there additional work needed?

I can drop the dprintk when I merge it.


> ---
> 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;
> +	}
> +
> 	nbl = find_or_allocate_block(lock_sop, &fp->fi_fhandle, nn);
> 	if (!nbl) {
> 		dprintk("NFSD: %s: unable to allocate block!\n", __func__);
> @@ -6542,13 +6549,6 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> 	file_lock->fl_end = last_byte_offset(lock->lk_offset, lock->lk_length);
> 	nfs4_transform_lock_offset(file_lock);
> 
> -	conflock = locks_alloc_lock();
> -	if (!conflock) {
> -		dprintk("NFSD: %s: unable to allocate lock!\n", __func__);
> -		status = nfserr_jukebox;
> -		goto out;
> -	}
> -
> 	if (fl_flags & FL_SLEEP) {
> 		nbl->nbl_time = jiffies;
> 		spin_lock(&nn->blocked_locks_lock);
> @@ -6581,17 +6581,15 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> 		status = nfserrno(err);
> 		break;
> 	}
> -out:
> -	if (nbl) {
> -		/* dequeue it if we queued it before */
> -		if (fl_flags & FL_SLEEP) {
> -			spin_lock(&nn->blocked_locks_lock);
> -			list_del_init(&nbl->nbl_list);
> -			list_del_init(&nbl->nbl_lru);
> -			spin_unlock(&nn->blocked_locks_lock);
> -		}
> -		free_blocked_lock(nbl);
> +	/* dequeue it if we queued it before */
> +	if (fl_flags & FL_SLEEP) {
> +		spin_lock(&nn->blocked_locks_lock);
> +		list_del_init(&nbl->nbl_list);
> +		list_del_init(&nbl->nbl_lru);
> +		spin_unlock(&nn->blocked_locks_lock);
> 	}
> +	free_blocked_lock(nbl);
> +out:
> 	if (nf)
> 		nfsd_file_put(nf);
> 	if (lock_stp) {
> -- 
> 2.17.1
> 

--
Chuck Lever




^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] nfsd: memory corruption in nfsd4_lock()
  2020-03-26 15:55 ` Chuck Lever
@ 2020-03-27  4:50   ` Vasily Averin
  2020-03-30 10:22     ` Jeff Layton
  0 siblings, 1 reply; 9+ messages in thread
From: Vasily Averin @ 2020-03-27  4:50 UTC (permalink / raw)
  To: J. Bruce Fields, Chuck Lever, Jeff Layton; +Cc: linux-nfs

Dear Chuck,
please use following patch instead.
-----
New struct nfsd4_blocked_lock allocated in find_or_allocate_block()
does not initialized nbl_list and nbl_lru.
If conflock allocation fails rollback can call list_del_init()
access uninitialized fields and corrupt memory.

v2: just initialize nbl_list and nbl_lru right after nbl allocation.

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 | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 369e574c5092..1b2eb6b35d64 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -266,6 +266,8 @@ find_or_allocate_block(struct nfs4_lockowner *lo, struct knfsd_fh *fh,
 	if (!nbl) {
 		nbl= kmalloc(sizeof(*nbl), GFP_KERNEL);
 		if (nbl) {
+			INIT_LIST_HEAD(&nbl->nbl_list);
+			INIT_LIST_HEAD(&nbl->nbl_lru);
 			fh_copy_shallow(&nbl->nbl_fh, fh);
 			locks_init_lock(&nbl->nbl_lock);
 			nfsd4_init_cb(&nbl->nbl_cb, lo->lo_owner.so_client,
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] nfsd: memory corruption in nfsd4_lock()
  2020-03-27  4:50   ` [PATCH v2] " Vasily Averin
@ 2020-03-30 10:22     ` Jeff Layton
  2020-03-30 14:52       ` Chuck Lever
  0 siblings, 1 reply; 9+ messages in thread
From: Jeff Layton @ 2020-03-30 10:22 UTC (permalink / raw)
  To: Vasily Averin, J. Bruce Fields, Chuck Lever; +Cc: linux-nfs

On Fri, 2020-03-27 at 07:50 +0300, Vasily Averin wrote:
> Dear Chuck,
> please use following patch instead.
> -----
> New struct nfsd4_blocked_lock allocated in find_or_allocate_block()
> does not initialized nbl_list and nbl_lru.
> If conflock allocation fails rollback can call list_del_init()
> access uninitialized fields and corrupt memory.
> 
> v2: just initialize nbl_list and nbl_lru right after nbl allocation.
> 
> 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 | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 369e574c5092..1b2eb6b35d64 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -266,6 +266,8 @@ find_or_allocate_block(struct nfs4_lockowner *lo, struct knfsd_fh *fh,
>  	if (!nbl) {
>  		nbl= kmalloc(sizeof(*nbl), GFP_KERNEL);
>  		if (nbl) {
> +			INIT_LIST_HEAD(&nbl->nbl_list);
> +			INIT_LIST_HEAD(&nbl->nbl_lru);
>  			fh_copy_shallow(&nbl->nbl_fh, fh);
>  			locks_init_lock(&nbl->nbl_lock);
>  			nfsd4_init_cb(&nbl->nbl_cb, lo->lo_owner.so_client,

Reviewed-by: Jeff Layton <jlayton@kernel.org>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] nfsd: memory corruption in nfsd4_lock()
  2020-03-30 10:22     ` Jeff Layton
@ 2020-03-30 14:52       ` Chuck Lever
  0 siblings, 0 replies; 9+ messages in thread
From: Chuck Lever @ 2020-03-30 14:52 UTC (permalink / raw)
  To: Vasily Averin; +Cc: Jeff Layton, Bruce Fields, Linux NFS Mailing List



> On Mar 30, 2020, at 6:22 AM, Jeff Layton <jlayton@kernel.org> wrote:
> 
> On Fri, 2020-03-27 at 07:50 +0300, Vasily Averin wrote:
>> Dear Chuck,
>> please use following patch instead.

Somehow this did not make it to my inbox on Friday, but Jeff's
Reviewed-by did show up today. I'll apply this one, thanks!


>> -----
>> New struct nfsd4_blocked_lock allocated in find_or_allocate_block()
>> does not initialized nbl_list and nbl_lru.
>> If conflock allocation fails rollback can call list_del_init()
>> access uninitialized fields and corrupt memory.
>> 
>> v2: just initialize nbl_list and nbl_lru right after nbl allocation.
>> 
>> 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 | 2 ++
>> 1 file changed, 2 insertions(+)
>> 
>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index 369e574c5092..1b2eb6b35d64 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c
>> @@ -266,6 +266,8 @@ find_or_allocate_block(struct nfs4_lockowner *lo, struct knfsd_fh *fh,
>> 	if (!nbl) {
>> 		nbl= kmalloc(sizeof(*nbl), GFP_KERNEL);
>> 		if (nbl) {
>> +			INIT_LIST_HEAD(&nbl->nbl_list);
>> +			INIT_LIST_HEAD(&nbl->nbl_lru);
>> 			fh_copy_shallow(&nbl->nbl_fh, fh);
>> 			locks_init_lock(&nbl->nbl_lock);
>> 			nfsd4_init_cb(&nbl->nbl_cb, lo->lo_owner.so_client,
> 
> Reviewed-by: Jeff Layton <jlayton@kernel.org>

--
Chuck Lever




^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-03-30 14:52 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-23  7:55 [PATCH] nfsd: memory corruption in nfsd4_lock() Vasily Averin
2020-03-23 12:18 ` Jeff Layton
2020-03-23 22:12   ` Vasily Averin
2020-03-23 13:50 ` Chuck Lever
2020-03-23 15:09   ` J. Bruce Fields
2020-03-26 15:55 ` Chuck Lever
2020-03-27  4:50   ` [PATCH v2] " Vasily Averin
2020-03-30 10:22     ` Jeff Layton
2020-03-30 14:52       ` Chuck Lever

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).