The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/4875

This e-mail was sent by the LXC bot, direct replies will not reach the author
unless they happen to be subscribed to this list.

=== Description (from pull-request) ===
When I suggested the `memcpy()` between ` vfs_cap_data` to `vfs_ns_cap_data` in order
to copy fscaps I forgot to mention that we need to ensure that all fields are
passed as 32 bit little endian integers. This commit makes sure to convert from
big to little endian when passing the uid down.

Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com>
From 4e0fe98e2a3e6d7a8256cb0aaa8fdd52a0758fdb Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brau...@ubuntu.com>
Date: Sat, 4 Aug 2018 14:35:53 +0200
Subject: [PATCH 1/3] idmap: C coding style fixups

Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com>
---
 shared/idmap/shift_linux.go | 42 +++++++++++++++++++------------------
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/shared/idmap/shift_linux.go b/shared/idmap/shift_linux.go
index 971227505b..4d8c47014f 100644
--- a/shared/idmap/shift_linux.go
+++ b/shared/idmap/shift_linux.go
@@ -48,10 +48,12 @@ struct vfs_ns_cap_data {
 };
 #endif
 
-int set_caps(char *path, char *caps, ssize_t len, uint32_t uid) {
+int set_caps(char *path, char *caps, ssize_t len, uint32_t uid)
+{
        ssize_t ret;
 
-       // Works because vfs_ns_cap_data is a superset of vfs_cap_data (rootid 
field added to the end)
+       // Works because vfs_ns_cap_data is a superset of vfs_cap_data (rootid
+       // field added to the end)
        struct vfs_ns_cap_data ns_xattr;
 
        memset(&ns_xattr, 0, sizeof(ns_xattr));
@@ -68,61 +70,61 @@ int set_caps(char *path, char *caps, ssize_t len, uint32_t 
uid) {
        return 0;
 }
 
-int shiftowner(char *basepath, char *path, int uid, int gid) {
+int shiftowner(char *basepath, char *path, int uid, int gid)
+{
+       int fd, ret;
+       char fdpath[PATH_MAX], realpath[PATH_MAX];
        struct stat sb;
-       int fd, r;
-       char fdpath[PATH_MAX];
-       char realpath[PATH_MAX];
 
-       fd = open(path, O_PATH|O_NOFOLLOW);
-       if (fd < 0 ) {
+       fd = open(path, O_PATH | O_NOFOLLOW);
+       if (fd < 0) {
                perror("Failed open");
                return 1;
        }
 
-       r = sprintf(fdpath, "/proc/self/fd/%d", fd);
-       if (r < 0) {
+       ret = sprintf(fdpath, "/proc/self/fd/%d", fd);
+       if (ret < 0) {
                perror("Failed sprintf");
                close(fd);
                return 1;
        }
 
-       r = readlink(fdpath, realpath, PATH_MAX);
-       if (r < 0) {
+       ret = readlink(fdpath, realpath, PATH_MAX);
+       if (ret < 0) {
                perror("Failed readlink");
                close(fd);
                return 1;
        }
 
        if (strlen(realpath) < strlen(basepath)) {
-               printf("Invalid path, source (%s) is outside of basepath 
(%s).\n", realpath, basepath);
+               printf("Invalid path, source (%s) is outside of basepath 
(%s)\n", realpath, basepath);
                close(fd);
                return 1;
        }
 
        if (strncmp(realpath, basepath, strlen(basepath))) {
-               printf("Invalid path, source (%s) is outside of basepath 
(%s).\n", realpath, basepath);
+               printf("Invalid path, source (%s) is outside of basepath " 
"(%s).\n", realpath, basepath);
                close(fd);
                return 1;
        }
 
-       r = fstat(fd, &sb);
-       if (r < 0) {
+       ret = fstat(fd, &sb);
+       if (ret < 0) {
                perror("Failed fstat");
                close(fd);
                return 1;
        }
 
-       r = fchownat(fd, "", uid, gid, AT_EMPTY_PATH|AT_SYMLINK_NOFOLLOW);
-       if (r < 0) {
+       ret = fchownat(fd, "", uid, gid, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW);
+       if (ret < 0) {
                perror("Failed chown");
                close(fd);
                return 1;
        }
 
        if (!S_ISLNK(sb.st_mode)) {
-               r = chmod(fdpath, sb.st_mode);
-               if (r < 0) {
+               ret = chmod(fdpath, sb.st_mode);
+               if (ret < 0) {
                        perror("Failed chmod");
                        close(fd);
                        return 1;

From 9ed277b84ca3e4fb169d5e102b3f627abb7401d0 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brau...@ubuntu.com>
Date: Sat, 4 Aug 2018 14:46:40 +0200
Subject: [PATCH 2/3] idmap: s/set_caps/set_vfs_ns_caps/g

Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com>
---
 shared/idmap/shift_linux.go | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/shared/idmap/shift_linux.go b/shared/idmap/shift_linux.go
index 4d8c47014f..253c5e7dcf 100644
--- a/shared/idmap/shift_linux.go
+++ b/shared/idmap/shift_linux.go
@@ -48,10 +48,9 @@ struct vfs_ns_cap_data {
 };
 #endif
 
-int set_caps(char *path, char *caps, ssize_t len, uint32_t uid)
-{
-       ssize_t ret;
 
+int set_vfs_ns_caps(char *path, char *caps, ssize_t len, uint32_t uid)
+{
        // Works because vfs_ns_cap_data is a superset of vfs_cap_data (rootid
        // field added to the end)
        struct vfs_ns_cap_data ns_xattr;
@@ -62,12 +61,7 @@ int set_caps(char *path, char *caps, ssize_t len, uint32_t 
uid)
        ns_xattr.magic_etc |= VFS_CAP_REVISION_3;
        ns_xattr.rootid = uid;
 
-       ret = setxattr(path, "security.capability", &ns_xattr, 
sizeof(ns_xattr), 0);
-       if (ret < 0) {
-               return -1;
-       }
-
-       return 0;
+       return setxattr(path, "security.capability", &ns_xattr, 
sizeof(ns_xattr), 0);
 }
 
 int shiftowner(char *basepath, char *path, int uid, int gid)
@@ -176,7 +170,7 @@ func SetCaps(path string, caps []byte, uid int64) error {
        ccaps := C.CString(string(caps))
        defer C.free(unsafe.Pointer(ccaps))
 
-       r := C.set_caps(cpath, ccaps, C.ssize_t(len(caps)), C.uint32_t(uid))
+       r := C.set_vfs_ns_caps(cpath, ccaps, C.ssize_t(len(caps)), 
C.uint32_t(uid))
        if r != 0 {
                return fmt.Errorf("Failed to apply capabilities to: %s", path)
        }

From 4c123ab4ad2691f577ab87dc29ff88acef29494d Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brau...@ubuntu.com>
Date: Sat, 4 Aug 2018 14:47:29 +0200
Subject: [PATCH 3/3] idmap: convert uid from big to little endian

When I suggested the memcpy() between vfs_cap_data to vfs_ns_cap_data in order
to copy fscaps I forgot to mention that we need to ensure that all fields are
passed as 32 bit little endian integers. This commit makes sure to convert from
big to little endian when passing the uid down.

Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com>
---
 shared/idmap/shift_linux.go | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/shared/idmap/shift_linux.go b/shared/idmap/shift_linux.go
index 253c5e7dcf..1899ec873f 100644
--- a/shared/idmap/shift_linux.go
+++ b/shared/idmap/shift_linux.go
@@ -13,6 +13,7 @@ import (
 // #cgo LDFLAGS: -lacl
 /*
 #define _GNU_SOURCE
+#include <byteswap.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <limits.h>
@@ -48,6 +49,11 @@ struct vfs_ns_cap_data {
 };
 #endif
 
+#if __BYTE_ORDER == __BIG_ENDIAN
+#define BE32_TO_LE32(x) bswap_32(x)
+#else
+#define BE32_TO_LE32(x) (x)
+#endif
 
 int set_vfs_ns_caps(char *path, char *caps, ssize_t len, uint32_t uid)
 {
@@ -59,7 +65,7 @@ int set_vfs_ns_caps(char *path, char *caps, ssize_t len, 
uint32_t uid)
        memcpy(&ns_xattr, caps, len);
        ns_xattr.magic_etc &= ~(VFS_CAP_REVISION_1 | VFS_CAP_REVISION_2);
        ns_xattr.magic_etc |= VFS_CAP_REVISION_3;
-       ns_xattr.rootid = uid;
+       ns_xattr.rootid = BE32_TO_LE32(uid);
 
        return setxattr(path, "security.capability", &ns_xattr, 
sizeof(ns_xattr), 0);
 }
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel

Reply via email to