qemu with hax to log dma reads & writes jcs.org/2018/11/12/vfio

9pfs: local: lremovexattr: don't follow symlinks

The local_lremovexattr() callback is vulnerable to symlink attacks because
it calls lremovexattr() which follows symbolic links in all path elements
but the rightmost one.

This patch introduces a helper to emulate the non-existing fremovexattrat()
function: it is implemented with /proc/self/fd which provides a trusted
path that can be safely passed to lremovexattr().

local_lremovexattr() is converted to use this helper and opendir_nofollow().

This partly fixes CVE-2016-9602.

Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

+36 -20
+2 -8
hw/9pfs/9p-posix-acl.c
··· 58 58 const char *path, const char *name) 59 59 { 60 60 int ret; 61 - char *buffer; 62 61 63 - buffer = rpath(ctx, path); 64 - ret = lremovexattr(buffer, MAP_ACL_ACCESS); 62 + ret = local_removexattr_nofollow(ctx, path, MAP_ACL_ACCESS); 65 63 if (ret == -1 && errno == ENODATA) { 66 64 /* 67 65 * We don't get ENODATA error when trying to remove a ··· 71 69 errno = 0; 72 70 ret = 0; 73 71 } 74 - g_free(buffer); 75 72 return ret; 76 73 } 77 74 ··· 111 108 const char *path, const char *name) 112 109 { 113 110 int ret; 114 - char *buffer; 115 111 116 - buffer = rpath(ctx, path); 117 - ret = lremovexattr(buffer, MAP_ACL_DEFAULT); 112 + ret = local_removexattr_nofollow(ctx, path, MAP_ACL_DEFAULT); 118 113 if (ret == -1 && errno == ENODATA) { 119 114 /* 120 115 * We don't get ENODATA error when trying to remove a ··· 124 119 errno = 0; 125 120 ret = 0; 126 121 } 127 - g_free(buffer); 128 122 return ret; 129 123 } 130 124
+1 -7
hw/9pfs/9p-xattr-user.c
··· 81 81 static int mp_user_removexattr(FsContext *ctx, 82 82 const char *path, const char *name) 83 83 { 84 - char *buffer; 85 - int ret; 86 - 87 84 if (strncmp(name, "user.virtfs.", 12) == 0) { 88 85 /* 89 86 * Don't allow fetch of user.virtfs namesapce ··· 92 89 errno = EACCES; 93 90 return -1; 94 91 } 95 - buffer = rpath(ctx, path); 96 - ret = lremovexattr(buffer, name); 97 - g_free(buffer); 98 - return ret; 92 + return local_removexattr_nofollow(ctx, path, name); 99 93 } 100 94 101 95 XattrOperations mapped_user_xattr = {
+31 -5
hw/9pfs/9p-xattr.c
··· 234 234 return local_setxattr_nofollow(ctx, path, name, value, size, flags); 235 235 } 236 236 237 - int pt_removexattr(FsContext *ctx, const char *path, const char *name) 237 + static ssize_t fremovexattrat_nofollow(int dirfd, const char *filename, 238 + const char *name) 238 239 { 239 - char *buffer; 240 + char *proc_path = g_strdup_printf("/proc/self/fd/%d/%s", dirfd, filename); 240 241 int ret; 241 242 242 - buffer = rpath(ctx, path); 243 - ret = lremovexattr(path, name); 244 - g_free(buffer); 243 + ret = lremovexattr(proc_path, name); 244 + g_free(proc_path); 245 245 return ret; 246 + } 247 + 248 + ssize_t local_removexattr_nofollow(FsContext *ctx, const char *path, 249 + const char *name) 250 + { 251 + char *dirpath = g_path_get_dirname(path); 252 + char *filename = g_path_get_basename(path); 253 + int dirfd; 254 + ssize_t ret = -1; 255 + 256 + dirfd = local_opendir_nofollow(ctx, dirpath); 257 + if (dirfd == -1) { 258 + goto out; 259 + } 260 + 261 + ret = fremovexattrat_nofollow(dirfd, filename, name); 262 + close_preserve_errno(dirfd); 263 + out: 264 + g_free(dirpath); 265 + g_free(filename); 266 + return ret; 267 + } 268 + 269 + int pt_removexattr(FsContext *ctx, const char *path, const char *name) 270 + { 271 + return local_removexattr_nofollow(ctx, path, name); 246 272 } 247 273 248 274 ssize_t notsup_getxattr(FsContext *ctx, const char *path, const char *name,
+2
hw/9pfs/9p-xattr.h
··· 34 34 ssize_t local_setxattr_nofollow(FsContext *ctx, const char *path, 35 35 const char *name, void *value, size_t size, 36 36 int flags); 37 + ssize_t local_removexattr_nofollow(FsContext *ctx, const char *path, 38 + const char *name); 37 39 38 40 extern XattrOperations mapped_user_xattr; 39 41 extern XattrOperations passthrough_user_xattr;