SRU request submitted:
https://lists.ubuntu.com/archives/kernel-team/2018-September/095580.html
** Changed in: linux (Ubuntu Cosmic)
Status: In Progress => Fix Committed
** Description changed:
Hey everyone,
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().
I have pushed a commit upstream that fixes this bug:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=82c9a927bc5df6e06b72d206d24a9d10cced4eb5
-
This commit passes the actual length of the xattr as returned by
vfs_getxattr() down.
A reproducer for the issue is:
- touch acl_posix
+ touch acl_posix
- setfacl -m user:0:rwx 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>
+ #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;
+ /* 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];
+ 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);
- }
+ 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);
- }
+ 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);
- }
+ 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;
+ 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;
- }
+ 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");
+ if (fret == EXIT_SUCCESS)
+ fprintf(stderr, "Test passed\n");
+ else
+ fprintf(stderr, "Test failed\n");
- _exit(fret);
- }
+ _exit(fret);
+ }
and run:
- ./tester acl_posix
+ ./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
+ 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
-
+ root@c1:~# ./t
+ Test passed
Please backport this to the 4.15 (bionic) and 4.4 (xenial) kernels. :)
Thanks!
Christian
** Description changed:
- Hey everyone,
+ == SRU Justification ==
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().
I have pushed a commit upstream that fixes this bug:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=82c9a927bc5df6e06b72d206d24a9d10cced4eb5
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
- Please backport this to the 4.15 (bionic) and 4.4 (xenial) kernels. :)
- Thanks!
- Christian
+ == Fix ==
+ 82c9a927bc5d ("getxattr: use correct xattr length")
+
+ == Regression Potential ==
+ Low. One liner that passes the actual length of the xattr as returned by
+ vfs_getxattr() down.
+
+ == Test Case ==
+ A test kernel was built with this patch and tested by the original bug
reporter.
+ The bug reporter states the test kernel resolved the bug.
--
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/1789746
Title:
getxattr: always handle namespaced attributes
To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1789746/+subscriptions
--
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
