linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/1] getxattr: use correct xattr length
@ 2018-06-07 11:43 Christian Brauner
  2018-06-07 11:43 ` [PATCH 1/1] " Christian Brauner
  0 siblings, 1 reply; 6+ messages in thread
From: Christian Brauner @ 2018-06-07 11:43 UTC (permalink / raw)
  To: linux-fsdevel, linux-kernel
  Cc: cjwatson, ebiederm, viro, serge, Christian Brauner

Hey,

When running in a container with a user namespace, if you call getxattr
with name = "system.posix_acl_access" and size % 8 != 4, then getxattr
silently skips the user namespace fixup that it normally does resulting in
un-fixed-up data being returned.
I think that the analysis that this is caused by
posix_acl_fix_xattr_to_user() being passed the total buffer size and not
the actual size of the xattr as returned by vfs_getxattr() is correct.
So this commit passes the actual length of the xattr as returned by
vfs_getxattr() down.

It might be worth considering making posix_acl_fix_xattr_userns() and
posix_acl_fix_xattr_to_user() return errors. Such that e.g.
posix_acl_xattr_count() errors can be caught in getxattr(). If that's
something we want I can send a follow-up patch.

More details of this can be found in
https://bugzilla.kernel.org/show_bug.cgi?id=199945

Thanks!
Christian

Christian Brauner (1):
  getxattr: use correct xattr length

 fs/xattr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.17.0

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

* [PATCH 1/1] getxattr: use correct xattr length
  2018-06-07 11:43 [PATCH 0/1] getxattr: use correct xattr length Christian Brauner
@ 2018-06-07 11:43 ` Christian Brauner
  2018-06-13 15:45   ` Serge E. Hallyn
  0 siblings, 1 reply; 6+ messages in thread
From: Christian Brauner @ 2018-06-07 11:43 UTC (permalink / raw)
  To: linux-fsdevel, linux-kernel
  Cc: cjwatson, ebiederm, viro, serge, Christian Brauner

When running in a container with a user namespace, if you call getxattr
with name = "system.posix_acl_access" and size % 8 != 4, then getxattr
silently skips the user namespace fixup that it normally does resulting in
un-fixed-up data being returned.
This is caused by posix_acl_fix_xattr_to_user() being passed the total
buffer size and not the actual size of the xattr as returned by
vfs_getxattr().
This commit passes the actual length of the xattr as returned by
vfs_getxattr() down.

A reproducer for the issue is:

  touch acl_posix

  setfacl -m user:0:rwx acl_posix

and the compile:

  #define _GNU_SOURCE
  #include <errno.h>
  #include <stdio.h>
  #include <stdlib.h>
  #include <string.h>
  #include <sys/types.h>
  #include <unistd.h>
  #include <attr/xattr.h>

  /* Run in user namespace with nsuid 0 mapped to uid != 0 on the host. */
  int main(int argc, void **argv)
  {
          ssize_t ret1, ret2;
          char buf1[128], buf2[132];
          int fret = EXIT_SUCCESS;
          char *file;

          if (argc < 2) {
                  fprintf(stderr,
                          "Please specify a file with "
                          "\"system.posix_acl_access\" permissions set\n");
                  _exit(EXIT_FAILURE);
          }
          file = argv[1];

          ret1 = getxattr(file, "system.posix_acl_access",
                          buf1, sizeof(buf1));
          if (ret1 < 0) {
                  fprintf(stderr, "%s - Failed to retrieve "
                                  "\"system.posix_acl_access\" "
                                  "from \"%s\"\n", strerror(errno), file);
                  _exit(EXIT_FAILURE);
          }

          ret2 = getxattr(file, "system.posix_acl_access",
                          buf2, sizeof(buf2));
          if (ret2 < 0) {
                  fprintf(stderr, "%s - Failed to retrieve "
                                  "\"system.posix_acl_access\" "
                                  "from \"%s\"\n", strerror(errno), file);
                  _exit(EXIT_FAILURE);
          }

          if (ret1 != ret2) {
                  fprintf(stderr, "The value of \"system.posix_acl_"
                                  "access\" for file \"%s\" changed "
                                  "between two successive calls\n", file);
                  _exit(EXIT_FAILURE);
          }

          for (ssize_t i = 0; i < ret2; i++) {
                  if (buf1[i] == buf2[i])
                          continue;

                  fprintf(stderr,
                          "Unexpected different in byte %zd: "
                          "%02x != %02x\n", i, buf1[i], buf2[i]);
                  fret = EXIT_FAILURE;
          }

          if (fret == EXIT_SUCCESS)
                  fprintf(stderr, "Test passed\n");
          else
                  fprintf(stderr, "Test failed\n");

          _exit(fret);
  }
and run:

  ./tester acl_posix

On a non-fixed up kernel this should return something like:

  root@c1:/# ./t
  Unexpected different in byte 16: ffffffa0 != 00
  Unexpected different in byte 17: ffffff86 != 00
  Unexpected different in byte 18: 01 != 00

and on a fixed kernel:

  root@c1:~# ./t
  Test passed

Link: https://bugzilla.kernel.org/show_bug.cgi?id=199945
Reported-by: Colin Watson <cjwatson@ubuntu.com>
Signed-off-by: Christian Brauner <christian@brauner.io>
---
 fs/xattr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xattr.c b/fs/xattr.c
index f9cb1db187b7..1bee74682513 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -539,7 +539,7 @@ getxattr(struct dentry *d, const char __user *name, void __user *value,
 	if (error > 0) {
 		if ((strcmp(kname, XATTR_NAME_POSIX_ACL_ACCESS) == 0) ||
 		    (strcmp(kname, XATTR_NAME_POSIX_ACL_DEFAULT) == 0))
-			posix_acl_fix_xattr_to_user(kvalue, size);
+			posix_acl_fix_xattr_to_user(kvalue, error);
 		if (size && copy_to_user(value, kvalue, error))
 			error = -EFAULT;
 	} else if (error == -ERANGE && size >= XATTR_SIZE_MAX) {
-- 
2.17.0

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

* Re: [PATCH 1/1] getxattr: use correct xattr length
  2018-06-07 11:43 ` [PATCH 1/1] " Christian Brauner
@ 2018-06-13 15:45   ` Serge E. Hallyn
  2018-06-13 16:42     ` Christian Brauner
  2018-07-23 15:00     ` Christian Brauner
  0 siblings, 2 replies; 6+ messages in thread
From: Serge E. Hallyn @ 2018-06-13 15:45 UTC (permalink / raw)
  To: Christian Brauner
  Cc: linux-fsdevel, linux-kernel, cjwatson, ebiederm, viro, serge

On Thu, Jun 07, 2018 at 01:43:48PM +0200, Christian Brauner wrote:
> When running in a container with a user namespace, if you call getxattr
> with name = "system.posix_acl_access" and size % 8 != 4, then getxattr
> silently skips the user namespace fixup that it normally does resulting in
> un-fixed-up data being returned.
> This is caused by posix_acl_fix_xattr_to_user() being passed the total
> buffer size and not the actual size of the xattr as returned by
> vfs_getxattr().
> This commit passes the actual length of the xattr as returned by
> vfs_getxattr() down.
> 
> A reproducer for the issue is:
> 
>   touch acl_posix
> 
>   setfacl -m user:0:rwx acl_posix
> 
> and the compile:
> 
>   #define _GNU_SOURCE
>   #include <errno.h>
>   #include <stdio.h>
>   #include <stdlib.h>
>   #include <string.h>
>   #include <sys/types.h>
>   #include <unistd.h>
>   #include <attr/xattr.h>
> 
>   /* Run in user namespace with nsuid 0 mapped to uid != 0 on the host. */
>   int main(int argc, void **argv)
>   {
>           ssize_t ret1, ret2;
>           char buf1[128], buf2[132];
>           int fret = EXIT_SUCCESS;
>           char *file;
> 
>           if (argc < 2) {
>                   fprintf(stderr,
>                           "Please specify a file with "
>                           "\"system.posix_acl_access\" permissions set\n");
>                   _exit(EXIT_FAILURE);
>           }
>           file = argv[1];
> 
>           ret1 = getxattr(file, "system.posix_acl_access",
>                           buf1, sizeof(buf1));
>           if (ret1 < 0) {
>                   fprintf(stderr, "%s - Failed to retrieve "
>                                   "\"system.posix_acl_access\" "
>                                   "from \"%s\"\n", strerror(errno), file);
>                   _exit(EXIT_FAILURE);
>           }
> 
>           ret2 = getxattr(file, "system.posix_acl_access",
>                           buf2, sizeof(buf2));
>           if (ret2 < 0) {
>                   fprintf(stderr, "%s - Failed to retrieve "
>                                   "\"system.posix_acl_access\" "
>                                   "from \"%s\"\n", strerror(errno), file);
>                   _exit(EXIT_FAILURE);
>           }
> 
>           if (ret1 != ret2) {
>                   fprintf(stderr, "The value of \"system.posix_acl_"
>                                   "access\" for file \"%s\" changed "
>                                   "between two successive calls\n", file);
>                   _exit(EXIT_FAILURE);
>           }
> 
>           for (ssize_t i = 0; i < ret2; i++) {
>                   if (buf1[i] == buf2[i])
>                           continue;
> 
>                   fprintf(stderr,
>                           "Unexpected different in byte %zd: "
>                           "%02x != %02x\n", i, buf1[i], buf2[i]);
>                   fret = EXIT_FAILURE;
>           }
> 
>           if (fret == EXIT_SUCCESS)
>                   fprintf(stderr, "Test passed\n");
>           else
>                   fprintf(stderr, "Test failed\n");
> 
>           _exit(fret);
>   }
> and run:
> 
>   ./tester acl_posix
> 
> On a non-fixed up kernel this should return something like:
> 
>   root@c1:/# ./t
>   Unexpected different in byte 16: ffffffa0 != 00
>   Unexpected different in byte 17: ffffff86 != 00
>   Unexpected different in byte 18: 01 != 00
> 
> and on a fixed kernel:
> 
>   root@c1:~# ./t
>   Test passed
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=199945
> Reported-by: Colin Watson <cjwatson@ubuntu.com>
> Signed-off-by: Christian Brauner <christian@brauner.io>

D'oh, sorry, I thought I had replied to this!

Acked-by: Serge Hallyn <serge@hallyn.com>

thanks,
Serge

> ---
>  fs/xattr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xattr.c b/fs/xattr.c
> index f9cb1db187b7..1bee74682513 100644
> --- a/fs/xattr.c
> +++ b/fs/xattr.c
> @@ -539,7 +539,7 @@ getxattr(struct dentry *d, const char __user *name, void __user *value,
>  	if (error > 0) {
>  		if ((strcmp(kname, XATTR_NAME_POSIX_ACL_ACCESS) == 0) ||
>  		    (strcmp(kname, XATTR_NAME_POSIX_ACL_DEFAULT) == 0))
> -			posix_acl_fix_xattr_to_user(kvalue, size);
> +			posix_acl_fix_xattr_to_user(kvalue, error);
>  		if (size && copy_to_user(value, kvalue, error))
>  			error = -EFAULT;
>  	} else if (error == -ERANGE && size >= XATTR_SIZE_MAX) {
> -- 
> 2.17.0

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

* Re: [PATCH 1/1] getxattr: use correct xattr length
  2018-06-13 15:45   ` Serge E. Hallyn
@ 2018-06-13 16:42     ` Christian Brauner
  2018-07-23 15:00     ` Christian Brauner
  1 sibling, 0 replies; 6+ messages in thread
From: Christian Brauner @ 2018-06-13 16:42 UTC (permalink / raw)
  To: Serge E. Hallyn; +Cc: linux-fsdevel, linux-kernel, cjwatson, ebiederm, viro

On Wed, Jun 13, 2018 at 10:45:37AM -0500, Serge Hallyn wrote:
> On Thu, Jun 07, 2018 at 01:43:48PM +0200, Christian Brauner wrote:
> > When running in a container with a user namespace, if you call getxattr
> > with name = "system.posix_acl_access" and size % 8 != 4, then getxattr
> > silently skips the user namespace fixup that it normally does resulting in
> > un-fixed-up data being returned.
> > This is caused by posix_acl_fix_xattr_to_user() being passed the total
> > buffer size and not the actual size of the xattr as returned by
> > vfs_getxattr().
> > This commit passes the actual length of the xattr as returned by
> > vfs_getxattr() down.
> > 
> > A reproducer for the issue is:
> > 
> >   touch acl_posix
> > 
> >   setfacl -m user:0:rwx acl_posix
> > 
> > and the compile:
> > 
> >   #define _GNU_SOURCE
> >   #include <errno.h>
> >   #include <stdio.h>
> >   #include <stdlib.h>
> >   #include <string.h>
> >   #include <sys/types.h>
> >   #include <unistd.h>
> >   #include <attr/xattr.h>
> > 
> >   /* Run in user namespace with nsuid 0 mapped to uid != 0 on the host. */
> >   int main(int argc, void **argv)
> >   {
> >           ssize_t ret1, ret2;
> >           char buf1[128], buf2[132];
> >           int fret = EXIT_SUCCESS;
> >           char *file;
> > 
> >           if (argc < 2) {
> >                   fprintf(stderr,
> >                           "Please specify a file with "
> >                           "\"system.posix_acl_access\" permissions set\n");
> >                   _exit(EXIT_FAILURE);
> >           }
> >           file = argv[1];
> > 
> >           ret1 = getxattr(file, "system.posix_acl_access",
> >                           buf1, sizeof(buf1));
> >           if (ret1 < 0) {
> >                   fprintf(stderr, "%s - Failed to retrieve "
> >                                   "\"system.posix_acl_access\" "
> >                                   "from \"%s\"\n", strerror(errno), file);
> >                   _exit(EXIT_FAILURE);
> >           }
> > 
> >           ret2 = getxattr(file, "system.posix_acl_access",
> >                           buf2, sizeof(buf2));
> >           if (ret2 < 0) {
> >                   fprintf(stderr, "%s - Failed to retrieve "
> >                                   "\"system.posix_acl_access\" "
> >                                   "from \"%s\"\n", strerror(errno), file);
> >                   _exit(EXIT_FAILURE);
> >           }
> > 
> >           if (ret1 != ret2) {
> >                   fprintf(stderr, "The value of \"system.posix_acl_"
> >                                   "access\" for file \"%s\" changed "
> >                                   "between two successive calls\n", file);
> >                   _exit(EXIT_FAILURE);
> >           }
> > 
> >           for (ssize_t i = 0; i < ret2; i++) {
> >                   if (buf1[i] == buf2[i])
> >                           continue;
> > 
> >                   fprintf(stderr,
> >                           "Unexpected different in byte %zd: "
> >                           "%02x != %02x\n", i, buf1[i], buf2[i]);
> >                   fret = EXIT_FAILURE;
> >           }
> > 
> >           if (fret == EXIT_SUCCESS)
> >                   fprintf(stderr, "Test passed\n");
> >           else
> >                   fprintf(stderr, "Test failed\n");
> > 
> >           _exit(fret);
> >   }
> > and run:
> > 
> >   ./tester acl_posix
> > 
> > On a non-fixed up kernel this should return something like:
> > 
> >   root@c1:/# ./t
> >   Unexpected different in byte 16: ffffffa0 != 00
> >   Unexpected different in byte 17: ffffff86 != 00
> >   Unexpected different in byte 18: 01 != 00
> > 
> > and on a fixed kernel:
> > 
> >   root@c1:~# ./t
> >   Test passed
> > 
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=199945
> > Reported-by: Colin Watson <cjwatson@ubuntu.com>
> > Signed-off-by: Christian Brauner <christian@brauner.io>
> 
> D'oh, sorry, I thought I had replied to this!

No problem at all. :)

Thanks!
Christian

> 
> Acked-by: Serge Hallyn <serge@hallyn.com>
> 
> thanks,
> Serge
> 
> > ---
> >  fs/xattr.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/xattr.c b/fs/xattr.c
> > index f9cb1db187b7..1bee74682513 100644
> > --- a/fs/xattr.c
> > +++ b/fs/xattr.c
> > @@ -539,7 +539,7 @@ getxattr(struct dentry *d, const char __user *name, void __user *value,
> >  	if (error > 0) {
> >  		if ((strcmp(kname, XATTR_NAME_POSIX_ACL_ACCESS) == 0) ||
> >  		    (strcmp(kname, XATTR_NAME_POSIX_ACL_DEFAULT) == 0))
> > -			posix_acl_fix_xattr_to_user(kvalue, size);
> > +			posix_acl_fix_xattr_to_user(kvalue, error);
> >  		if (size && copy_to_user(value, kvalue, error))
> >  			error = -EFAULT;
> >  	} else if (error == -ERANGE && size >= XATTR_SIZE_MAX) {
> > -- 
> > 2.17.0

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

* Re: [PATCH 1/1] getxattr: use correct xattr length
  2018-06-13 15:45   ` Serge E. Hallyn
  2018-06-13 16:42     ` Christian Brauner
@ 2018-07-23 15:00     ` Christian Brauner
  2018-07-23 15:14       ` Serge E. Hallyn
  1 sibling, 1 reply; 6+ messages in thread
From: Christian Brauner @ 2018-07-23 15:00 UTC (permalink / raw)
  To: Serge E. Hallyn; +Cc: linux-fsdevel, linux-kernel, cjwatson, ebiederm, viro

On Wed, Jun 13, 2018 at 10:45:37AM -0500, Serge Hallyn wrote:
> On Thu, Jun 07, 2018 at 01:43:48PM +0200, Christian Brauner wrote:
> > When running in a container with a user namespace, if you call getxattr
> > with name = "system.posix_acl_access" and size % 8 != 4, then getxattr
> > silently skips the user namespace fixup that it normally does resulting in
> > un-fixed-up data being returned.
> > This is caused by posix_acl_fix_xattr_to_user() being passed the total
> > buffer size and not the actual size of the xattr as returned by
> > vfs_getxattr().
> > This commit passes the actual length of the xattr as returned by
> > vfs_getxattr() down.
> > 
> > A reproducer for the issue is:
> > 
> >   touch acl_posix
> > 
> >   setfacl -m user:0:rwx acl_posix
> > 
> > and the compile:
> > 
> >   #define _GNU_SOURCE
> >   #include <errno.h>
> >   #include <stdio.h>
> >   #include <stdlib.h>
> >   #include <string.h>
> >   #include <sys/types.h>
> >   #include <unistd.h>
> >   #include <attr/xattr.h>
> > 
> >   /* Run in user namespace with nsuid 0 mapped to uid != 0 on the host. */
> >   int main(int argc, void **argv)
> >   {
> >           ssize_t ret1, ret2;
> >           char buf1[128], buf2[132];
> >           int fret = EXIT_SUCCESS;
> >           char *file;
> > 
> >           if (argc < 2) {
> >                   fprintf(stderr,
> >                           "Please specify a file with "
> >                           "\"system.posix_acl_access\" permissions set\n");
> >                   _exit(EXIT_FAILURE);
> >           }
> >           file = argv[1];
> > 
> >           ret1 = getxattr(file, "system.posix_acl_access",
> >                           buf1, sizeof(buf1));
> >           if (ret1 < 0) {
> >                   fprintf(stderr, "%s - Failed to retrieve "
> >                                   "\"system.posix_acl_access\" "
> >                                   "from \"%s\"\n", strerror(errno), file);
> >                   _exit(EXIT_FAILURE);
> >           }
> > 
> >           ret2 = getxattr(file, "system.posix_acl_access",
> >                           buf2, sizeof(buf2));
> >           if (ret2 < 0) {
> >                   fprintf(stderr, "%s - Failed to retrieve "
> >                                   "\"system.posix_acl_access\" "
> >                                   "from \"%s\"\n", strerror(errno), file);
> >                   _exit(EXIT_FAILURE);
> >           }
> > 
> >           if (ret1 != ret2) {
> >                   fprintf(stderr, "The value of \"system.posix_acl_"
> >                                   "access\" for file \"%s\" changed "
> >                                   "between two successive calls\n", file);
> >                   _exit(EXIT_FAILURE);
> >           }
> > 
> >           for (ssize_t i = 0; i < ret2; i++) {
> >                   if (buf1[i] == buf2[i])
> >                           continue;
> > 
> >                   fprintf(stderr,
> >                           "Unexpected different in byte %zd: "
> >                           "%02x != %02x\n", i, buf1[i], buf2[i]);
> >                   fret = EXIT_FAILURE;
> >           }
> > 
> >           if (fret == EXIT_SUCCESS)
> >                   fprintf(stderr, "Test passed\n");
> >           else
> >                   fprintf(stderr, "Test failed\n");
> > 
> >           _exit(fret);
> >   }
> > and run:
> > 
> >   ./tester acl_posix
> > 
> > On a non-fixed up kernel this should return something like:
> > 
> >   root@c1:/# ./t
> >   Unexpected different in byte 16: ffffffa0 != 00
> >   Unexpected different in byte 17: ffffff86 != 00
> >   Unexpected different in byte 18: 01 != 00
> > 
> > and on a fixed kernel:
> > 
> >   root@c1:~# ./t
> >   Test passed
> > 
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=199945
> > Reported-by: Colin Watson <cjwatson@ubuntu.com>
> > Signed-off-by: Christian Brauner <christian@brauner.io>
> 
> D'oh, sorry, I thought I had replied to this!
> 
> Acked-by: Serge Hallyn <serge@hallyn.com>

Should this still land as a bugfix in 4.18?

Christian

> 
> thanks,
> Serge
> 
> > ---
> >  fs/xattr.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/xattr.c b/fs/xattr.c
> > index f9cb1db187b7..1bee74682513 100644
> > --- a/fs/xattr.c
> > +++ b/fs/xattr.c
> > @@ -539,7 +539,7 @@ getxattr(struct dentry *d, const char __user *name, void __user *value,
> >  	if (error > 0) {
> >  		if ((strcmp(kname, XATTR_NAME_POSIX_ACL_ACCESS) == 0) ||
> >  		    (strcmp(kname, XATTR_NAME_POSIX_ACL_DEFAULT) == 0))
> > -			posix_acl_fix_xattr_to_user(kvalue, size);
> > +			posix_acl_fix_xattr_to_user(kvalue, error);
> >  		if (size && copy_to_user(value, kvalue, error))
> >  			error = -EFAULT;
> >  	} else if (error == -ERANGE && size >= XATTR_SIZE_MAX) {
> > -- 
> > 2.17.0

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

* Re: [PATCH 1/1] getxattr: use correct xattr length
  2018-07-23 15:00     ` Christian Brauner
@ 2018-07-23 15:14       ` Serge E. Hallyn
  0 siblings, 0 replies; 6+ messages in thread
From: Serge E. Hallyn @ 2018-07-23 15:14 UTC (permalink / raw)
  To: Christian Brauner
  Cc: Serge E. Hallyn, linux-fsdevel, linux-kernel, cjwatson, ebiederm, viro

Quoting Christian Brauner (christian.brauner@canonical.com):
> On Wed, Jun 13, 2018 at 10:45:37AM -0500, Serge Hallyn wrote:
> > On Thu, Jun 07, 2018 at 01:43:48PM +0200, Christian Brauner wrote:
> > > When running in a container with a user namespace, if you call getxattr
> > > with name = "system.posix_acl_access" and size % 8 != 4, then getxattr
> > > silently skips the user namespace fixup that it normally does resulting in
> > > un-fixed-up data being returned.
> > > This is caused by posix_acl_fix_xattr_to_user() being passed the total
> > > buffer size and not the actual size of the xattr as returned by
> > > vfs_getxattr().
> > > This commit passes the actual length of the xattr as returned by
> > > vfs_getxattr() down.
> > > 
> > > A reproducer for the issue is:
> > > 
> > >   touch acl_posix
> > > 
> > >   setfacl -m user:0:rwx acl_posix
> > > 
> > > and the compile:
> > > 
> > >   #define _GNU_SOURCE
> > >   #include <errno.h>
> > >   #include <stdio.h>
> > >   #include <stdlib.h>
> > >   #include <string.h>
> > >   #include <sys/types.h>
> > >   #include <unistd.h>
> > >   #include <attr/xattr.h>
> > > 
> > >   /* Run in user namespace with nsuid 0 mapped to uid != 0 on the host. */
> > >   int main(int argc, void **argv)
> > >   {
> > >           ssize_t ret1, ret2;
> > >           char buf1[128], buf2[132];
> > >           int fret = EXIT_SUCCESS;
> > >           char *file;
> > > 
> > >           if (argc < 2) {
> > >                   fprintf(stderr,
> > >                           "Please specify a file with "
> > >                           "\"system.posix_acl_access\" permissions set\n");
> > >                   _exit(EXIT_FAILURE);
> > >           }
> > >           file = argv[1];
> > > 
> > >           ret1 = getxattr(file, "system.posix_acl_access",
> > >                           buf1, sizeof(buf1));
> > >           if (ret1 < 0) {
> > >                   fprintf(stderr, "%s - Failed to retrieve "
> > >                                   "\"system.posix_acl_access\" "
> > >                                   "from \"%s\"\n", strerror(errno), file);
> > >                   _exit(EXIT_FAILURE);
> > >           }
> > > 
> > >           ret2 = getxattr(file, "system.posix_acl_access",
> > >                           buf2, sizeof(buf2));
> > >           if (ret2 < 0) {
> > >                   fprintf(stderr, "%s - Failed to retrieve "
> > >                                   "\"system.posix_acl_access\" "
> > >                                   "from \"%s\"\n", strerror(errno), file);
> > >                   _exit(EXIT_FAILURE);
> > >           }
> > > 
> > >           if (ret1 != ret2) {
> > >                   fprintf(stderr, "The value of \"system.posix_acl_"
> > >                                   "access\" for file \"%s\" changed "
> > >                                   "between two successive calls\n", file);
> > >                   _exit(EXIT_FAILURE);
> > >           }
> > > 
> > >           for (ssize_t i = 0; i < ret2; i++) {
> > >                   if (buf1[i] == buf2[i])
> > >                           continue;
> > > 
> > >                   fprintf(stderr,
> > >                           "Unexpected different in byte %zd: "
> > >                           "%02x != %02x\n", i, buf1[i], buf2[i]);
> > >                   fret = EXIT_FAILURE;
> > >           }
> > > 
> > >           if (fret == EXIT_SUCCESS)
> > >                   fprintf(stderr, "Test passed\n");
> > >           else
> > >                   fprintf(stderr, "Test failed\n");
> > > 
> > >           _exit(fret);
> > >   }
> > > and run:
> > > 
> > >   ./tester acl_posix
> > > 
> > > On a non-fixed up kernel this should return something like:
> > > 
> > >   root@c1:/# ./t
> > >   Unexpected different in byte 16: ffffffa0 != 00
> > >   Unexpected different in byte 17: ffffff86 != 00
> > >   Unexpected different in byte 18: 01 != 00
> > > 
> > > and on a fixed kernel:
> > > 
> > >   root@c1:~# ./t
> > >   Test passed
> > > 
> > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=199945
> > > Reported-by: Colin Watson <cjwatson@ubuntu.com>
> > > Signed-off-by: Christian Brauner <christian@brauner.io>
> > 
> > D'oh, sorry, I thought I had replied to this!
> > 
> > Acked-by: Serge Hallyn <serge@hallyn.com>
> 
> Should this still land as a bugfix in 4.18?

Eric, do you mind picking this up?

> Christian
> 
> > 
> > thanks,
> > Serge
> > 
> > > ---
> > >  fs/xattr.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/fs/xattr.c b/fs/xattr.c
> > > index f9cb1db187b7..1bee74682513 100644
> > > --- a/fs/xattr.c
> > > +++ b/fs/xattr.c
> > > @@ -539,7 +539,7 @@ getxattr(struct dentry *d, const char __user *name, void __user *value,
> > >  	if (error > 0) {
> > >  		if ((strcmp(kname, XATTR_NAME_POSIX_ACL_ACCESS) == 0) ||
> > >  		    (strcmp(kname, XATTR_NAME_POSIX_ACL_DEFAULT) == 0))
> > > -			posix_acl_fix_xattr_to_user(kvalue, size);
> > > +			posix_acl_fix_xattr_to_user(kvalue, error);
> > >  		if (size && copy_to_user(value, kvalue, error))
> > >  			error = -EFAULT;
> > >  	} else if (error == -ERANGE && size >= XATTR_SIZE_MAX) {
> > > -- 
> > > 2.17.0

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

end of thread, other threads:[~2018-07-23 16:15 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-07 11:43 [PATCH 0/1] getxattr: use correct xattr length Christian Brauner
2018-06-07 11:43 ` [PATCH 1/1] " Christian Brauner
2018-06-13 15:45   ` Serge E. Hallyn
2018-06-13 16:42     ` Christian Brauner
2018-07-23 15:00     ` Christian Brauner
2018-07-23 15:14       ` Serge E. Hallyn

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