diff mbox

[Quantal,SRU] (upstream) NFSv4: Fix the string length returned by the idmapper

Message ID 1365026247-30217-1-git-send-email-chiluk@canonical.com
State New
Headers show

Commit Message

Dave Chiluk April 3, 2013, 9:57 p.m. UTC
BugLink: http://bugs.launchpad.net/bugs/1101292

Functions like nfs_map_uid_to_name() and nfs_map_gid_to_group() are
expected to return a string without any terminating NUL character.
Regression introduced by commit 57e62324e469e092ecc6c94a7a86fe4bd6ac5172
(NFS: Store the legacy idmapper result in the keyring).

This fixes inability to chown or chgrp files on AIX nfs shares.

Reported-by: Dave Chiluk <chiluk@canonical.com>
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: Bryan Schumaker <bjschuma@netapp.com>
Cc: stable@vger.kernel.org [>=3.4]
(backported from commit 3070d4a2f8aa4ac06f424eb497637d41d049bb3a linux)

Signed-off-by: Dave Chiluk <chiluk@canonical.com>
---
 fs/nfs/idmap.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)
---
 fs/nfs/idmap.c |   13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

Comments

Luis Henriques April 4, 2013, 8:45 a.m. UTC | #1
On Wed, Apr 03, 2013 at 04:57:27PM -0500, Dave Chiluk wrote:
> BugLink: http://bugs.launchpad.net/bugs/1101292
> 
> Functions like nfs_map_uid_to_name() and nfs_map_gid_to_group() are
> expected to return a string without any terminating NUL character.
> Regression introduced by commit 57e62324e469e092ecc6c94a7a86fe4bd6ac5172
> (NFS: Store the legacy idmapper result in the keyring).
> 
> This fixes inability to chown or chgrp files on AIX nfs shares.
> 
> Reported-by: Dave Chiluk <chiluk@canonical.com>
> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> Cc: Bryan Schumaker <bjschuma@netapp.com>
> Cc: stable@vger.kernel.org [>=3.4]
> (backported from commit 3070d4a2f8aa4ac06f424eb497637d41d049bb3a linux)

This commit has been queued for 3.5 stable kernel (should be in
3.5.7.10) and the backport seems to be correct (or at least its not
different from my own backport...)

Also, there's positive tests results.

Cheers,
--
Luis

> 
> Signed-off-by: Dave Chiluk <chiluk@canonical.com>
> ---
>  fs/nfs/idmap.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> ---
>  fs/nfs/idmap.c |   13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/nfs/idmap.c b/fs/nfs/idmap.c
> index a0972e9..e4476dc 100644
> --- a/fs/nfs/idmap.c
> +++ b/fs/nfs/idmap.c
> @@ -710,9 +710,9 @@ out1:
>  	return ret;
>  }
>  
> -static int nfs_idmap_instantiate(struct key *key, struct key *authkey, char *data)
> +static int nfs_idmap_instantiate(struct key *key, struct key *authkey, char *data, size_t datalen)
>  {
> -	return key_instantiate_and_link(key, data, strlen(data) + 1,
> +	return key_instantiate_and_link(key, data, datalen,
>  					id_resolver_cache->thread_keyring,
>  					authkey);
>  }
> @@ -720,15 +720,18 @@ static int nfs_idmap_instantiate(struct key *key, struct key *authkey, char *dat
>  static int nfs_idmap_read_message(struct idmap_msg *im, struct key *key, struct key *authkey)
>  {
>  	char id_str[NFS_UINT_MAXLEN];
> +	size_t len;
>  	int ret = -EINVAL;
>  
>  	switch (im->im_conv) {
>  	case IDMAP_CONV_NAMETOID:
> -		sprintf(id_str, "%d", im->im_id);
> -		ret = nfs_idmap_instantiate(key, authkey, id_str);
> +		/* Note: here we store the NUL terminator too */
> +		len = sprintf(id_str, "%d", im->im_id) + 1;
> +		ret = nfs_idmap_instantiate(key, authkey, id_str, len);
>  		break;
>  	case IDMAP_CONV_IDTONAME:
> -		ret = nfs_idmap_instantiate(key, authkey, im->im_name);
> +		len = strlen(im->im_name);
> +		ret = nfs_idmap_instantiate(key, authkey, im->im_name, len);
>  		break;
>  	}
>  
> -- 
> 1.7.9.5
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Tim Gardner April 4, 2013, 12:36 p.m. UTC | #2
Applied after first fixing authorship and the upstream commit SHA1 from
which this patch was back ported.

Luis - you might check your 3.5.y patch to make sure its back ported
from cf4ab538f1516606d3ae730dce15d6f33d96b7e1

rtg
Luis Henriques April 4, 2013, 12:52 p.m. UTC | #3
On Thu, Apr 04, 2013 at 06:36:31AM -0600, Tim Gardner wrote:
> Applied after first fixing authorship and the upstream commit SHA1 from
> which this patch was back ported.

Ups, I didn't relalised it had the wrong sha1.

> Luis - you might check your 3.5.y patch to make sure its back ported
> from cf4ab538f1516606d3ae730dce15d6f33d96b7e1

Thanks, I checked and 3.5 has the correct sha1.

Cheers,
--
Luis
Dave Chiluk April 4, 2013, 1:54 p.m. UTC | #4
Hmm, I'm not sure how that happened.  Maybe I must have used the sha
from the commit that I applied from the mailing list instead of the
actual upstream commit.  In both cases the commits at least in my repos
match.


On 04/04/2013 07:52 AM, Luis Henriques wrote:
> On Thu, Apr 04, 2013 at 06:36:31AM -0600, Tim Gardner wrote:
>> Applied after first fixing authorship and the upstream commit SHA1 from
>> which this patch was back ported.
> 
> Ups, I didn't relalised it had the wrong sha1.
> 
>> Luis - you might check your 3.5.y patch to make sure its back ported
>> from cf4ab538f1516606d3ae730dce15d6f33d96b7e1
> 
> Thanks, I checked and 3.5 has the correct sha1.
> 
> Cheers,
> --
> Luis
>
diff mbox

Patch

diff --git a/fs/nfs/idmap.c b/fs/nfs/idmap.c
index a0972e9..e4476dc 100644
--- a/fs/nfs/idmap.c
+++ b/fs/nfs/idmap.c
@@ -710,9 +710,9 @@  out1:
 	return ret;
 }
 
-static int nfs_idmap_instantiate(struct key *key, struct key *authkey, char *data)
+static int nfs_idmap_instantiate(struct key *key, struct key *authkey, char *data, size_t datalen)
 {
-	return key_instantiate_and_link(key, data, strlen(data) + 1,
+	return key_instantiate_and_link(key, data, datalen,
 					id_resolver_cache->thread_keyring,
 					authkey);
 }
@@ -720,15 +720,18 @@  static int nfs_idmap_instantiate(struct key *key, struct key *authkey, char *dat
 static int nfs_idmap_read_message(struct idmap_msg *im, struct key *key, struct key *authkey)
 {
 	char id_str[NFS_UINT_MAXLEN];
+	size_t len;
 	int ret = -EINVAL;
 
 	switch (im->im_conv) {
 	case IDMAP_CONV_NAMETOID:
-		sprintf(id_str, "%d", im->im_id);
-		ret = nfs_idmap_instantiate(key, authkey, id_str);
+		/* Note: here we store the NUL terminator too */
+		len = sprintf(id_str, "%d", im->im_id) + 1;
+		ret = nfs_idmap_instantiate(key, authkey, id_str, len);
 		break;
 	case IDMAP_CONV_IDTONAME:
-		ret = nfs_idmap_instantiate(key, authkey, im->im_name);
+		len = strlen(im->im_name);
+		ret = nfs_idmap_instantiate(key, authkey, im->im_name, len);
 		break;
 	}