2009-03-31 21:02:45

by Greg Banks

[permalink] [raw]
Subject: [patch 15/29] knfsd: fix reply cache memory corruption

Fix a regression in the reply cache introduced when the code was
converted to use proper Linux lists. When a new entry needs to be
inserted, the case where all the entries are currently being used
by threads is not correctly detected. This can result in memory
corruption and a crash. In the current code this is an extremely
unlikely corner case; it would require the machine to have 1024
nfsd threads and all of them to be busy at the same time. However,
upcoming reply cache changes make this more likely; a crash due to
this problem was actually observed in field.

Signed-off-by: Greg Banks <[email protected]>
---

fs/nfsd/nfscache.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

Index: bfields/fs/nfsd/nfscache.c
===================================================================
--- bfields.orig/fs/nfsd/nfscache.c
+++ bfields/fs/nfsd/nfscache.c
@@ -177,8 +177,8 @@ nfsd_cache_lookup(struct svc_rqst *rqstp
}
}

- /* This should not happen */
- if (rp == NULL) {
+ /* All entries on the LRU are in-progress. This should not happen */
+ if (&rp->c_lru == &lru_head) {
static int complaints;

printk(KERN_WARNING "nfsd: all repcache entries locked!\n");

--
Greg


2009-05-12 19:55:56

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [patch 15/29] knfsd: fix reply cache memory corruption

On Wed, Apr 01, 2009 at 07:28:15AM +1100, Greg Banks wrote:
> Fix a regression in the reply cache introduced when the code was
> converted to use proper Linux lists. When a new entry needs to be
> inserted, the case where all the entries are currently being used
> by threads is not correctly detected. This can result in memory
> corruption and a crash. In the current code this is an extremely
> unlikely corner case; it would require the machine to have 1024
> nfsd threads and all of them to be busy at the same time. However,
> upcoming reply cache changes make this more likely; a crash due to
> this problem was actually observed in field.

OK, that does indeed sound hard to reproduce as is, but may as well
apply it for 2.6.31 now.--b.

>
> Signed-off-by: Greg Banks <[email protected]>
> ---
>
> fs/nfsd/nfscache.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> Index: bfields/fs/nfsd/nfscache.c
> ===================================================================
> --- bfields.orig/fs/nfsd/nfscache.c
> +++ bfields/fs/nfsd/nfscache.c
> @@ -177,8 +177,8 @@ nfsd_cache_lookup(struct svc_rqst *rqstp
> }
> }
>
> - /* This should not happen */
> - if (rp == NULL) {
> + /* All entries on the LRU are in-progress. This should not happen */
> + if (&rp->c_lru == &lru_head) {
> static int complaints;
>
> printk(KERN_WARNING "nfsd: all repcache entries locked!\n");
>
> --
> Greg