[PATCH] nfs: don't pass non-NULL-terminated string to pr_notice()

[PATCH] nfs: don't pass non-NULL-terminated string to pr_notice()

[Jeff Layton]

There is no guarantee that the strings in the nfs_cache_array will be
NULL-terminated. In the event that we end up hitting a readdir loop, we
need to ensure that we pass the warning message a properly-terminated
string.

Reported-by: Lachlan McIlroy <lmcilroy@redhat.com>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/nfs/dir.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 5c0b6ecc3a88..4689b125f9fe 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -304,12 +304,13 @@ int nfs_readdir_search_for_cookie(struct nfs_cache_array *array, nfs_readdir_des
 				if (ctx->duped > 0
 				    && ctx->dup_cookie == *desc->dir_cookie) {
 					if (printk_ratelimit()) {
+						char *name = kstrndup(array->array[i].string.name, array->array[i].string.len, GFP_KERNEL);
+
 						pr_notice("NFS: directory %pD2 contains a readdir loop."
 								"Please contact your server vendor.  "
 								"The file: %s has duplicate cookie %llu\n",
-								desc->file,
-								array->array[i].string.name,
-								*desc->dir_cookie);
+								desc->file, name, *desc->dir_cookie);
+						kfree(name);
 					}
 					status = -ELOOP;
 					goto out;
-- 
1.9.0

Re: [PATCH] nfs: don't pass non-NULL-terminated string to pr_notice()

[Trond Myklebust]

On Apr 4, 2014, at 6:00, Jeff Layton <jlayton@redhat.com> wrote:

> There is no guarantee that the strings in the nfs_cache_array will be
> NULL-terminated. In the event that we end up hitting a readdir loop, we
> need to ensure that we pass the warning message a properly-terminated
> string.
> 
> Reported-by: Lachlan McIlroy <lmcilroy@redhat.com>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
> fs/nfs/dir.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> index 5c0b6ecc3a88..4689b125f9fe 100644
> --- a/fs/nfs/dir.c
> +++ b/fs/nfs/dir.c
> @@ -304,12 +304,13 @@ int nfs_readdir_search_for_cookie(struct nfs_cache_array *array, nfs_readdir_des
> 				if (ctx->duped > 0
> 				    && ctx->dup_cookie == *desc->dir_cookie) {
> 					if (printk_ratelimit()) {
> +						char *name = kstrndup(array->array[i].string.name, array->array[i].string.len, GFP_KERNEL);
> +
> 						pr_notice("NFS: directory %pD2 contains a readdir loop."
> 								"Please contact your server vendor.  "
> 								"The file: %s has duplicate cookie %llu\n",
> -								desc->file,
> -								array->array[i].string.name,
> -								*desc->dir_cookie);
> +								desc->file, name, *desc->dir_cookie);
> +						kfree(name);
> 					}

Umm… Any reason why we couldn’t just use ‘%.*s’ ?

_________________________________
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@primarydata.com

Re: [PATCH] nfs: don't pass non-NULL-terminated string to pr_notice()

[Jeff Layton]

On Fri, 4 Apr 2014 09:51:11 -0400
Trond Myklebust <trond.myklebust@primarydata.com> wrote:

> 
> On Apr 4, 2014, at 6:00, Jeff Layton <jlayton@redhat.com> wrote:
> 
> > There is no guarantee that the strings in the nfs_cache_array will be
> > NULL-terminated. In the event that we end up hitting a readdir loop, we
> > need to ensure that we pass the warning message a properly-terminated
> > string.
> > 
> > Reported-by: Lachlan McIlroy <lmcilroy@redhat.com>
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> > fs/nfs/dir.c | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> > index 5c0b6ecc3a88..4689b125f9fe 100644
> > --- a/fs/nfs/dir.c
> > +++ b/fs/nfs/dir.c
> > @@ -304,12 +304,13 @@ int nfs_readdir_search_for_cookie(struct nfs_cache_array *array, nfs_readdir_des
> > 				if (ctx->duped > 0
> > 				    && ctx->dup_cookie == *desc->dir_cookie) {
> > 					if (printk_ratelimit()) {
> > +						char *name = kstrndup(array->array[i].string.name, array->array[i].string.len, GFP_KERNEL);
> > +
> > 						pr_notice("NFS: directory %pD2 contains a readdir loop."
> > 								"Please contact your server vendor.  "
> > 								"The file: %s has duplicate cookie %llu\n",
> > -								desc->file,
> > -								array->array[i].string.name,
> > -								*desc->dir_cookie);
> > +								desc->file, name, *desc->dir_cookie);
> > +						kfree(name);
> > 					}
> 
> Umm… Any reason why we couldn’t just use ‘%.*s’ ?
> 

No reason at all. I had never run across that...

I'll respin and repost -- thanks!
-- 
Jeff Layton <jlayton@redhat.com>

[PATCH v2] nfs: pass string length to pr_notice message about readdir loops

[Jeff Layton]

There is no guarantee that the strings in the nfs_cache_array will be
NULL-terminated. In the event that we end up hitting a readdir loop, we
need to ensure that we pass the warning message the length of the
string.

Reported-by: Lachlan McIlroy <lmcilroy@redhat.com>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/nfs/dir.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index e6f8e2c97653..0013d5fc1aed 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -306,10 +306,9 @@ int nfs_readdir_search_for_cookie(struct nfs_cache_array *array, nfs_readdir_des
 					if (printk_ratelimit()) {
 						pr_notice("NFS: directory %pD2 contains a readdir loop."
 								"Please contact your server vendor.  "
-								"The file: %s has duplicate cookie %llu\n",
-								desc->file,
-								array->array[i].string.name,
-								*desc->dir_cookie);
+								"The file: %.*s has duplicate cookie %llu\n",
+								desc->file, array->array[i].string.len,
+								array->array[i].string.name, *desc->dir_cookie);
 					}
 					status = -ELOOP;
 					goto out;
-- 
1.9.0