aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGreg Kurz <groug@kaod.org>2017-05-25 10:30:13 +0200
committerGreg Kurz <groug@kaod.org>2017-05-25 10:30:13 +0200
commit6a87e7929f97b86c5823d4616fa1aa7636b2f116 (patch)
treea7d2ae140c3ab125403e14e0a70b982b7a8985ec
parenta17d8659c466aa2470fdf5b05c88e9e68d40d6ee (diff)
9pfs: local: fix unlink of alien files in mapped-file mode
When trying to remove a file from a directory, both created in non-mapped mode, the file remains and EBADF is returned to the guest. This is a regression introduced by commit "df4938a6651b 9pfs: local: unlinkat: don't follow symlinks" when fixing CVE-2016-9602. It changed the way we unlink the metadata file from ret = remove("$dir/.virtfs_metadata/$name"); if (ret < 0 && errno != ENOENT) { /* Error out */ } /* Ignore absence of metadata */ to fd = openat("$dir/.virtfs_metadata") unlinkat(fd, "$name") if (ret < 0 && errno != ENOENT) { /* Error out */ } /* Ignore absence of metadata */ If $dir was created in non-mapped mode, openat() fails with ENOENT and we pass -1 to unlinkat(), which fails in turn with EBADF. We just need to check the return of openat() and ignore ENOENT, in order to restore the behaviour we had with remove(). Signed-off-by: Greg Kurz <groug@kaod.org> Reviewed-by: Eric Blake <eblake@redhat.com> [groug: rewrote the comments as suggested by Eric]
-rw-r--r--hw/9pfs/9p-local.c34
1 files changed, 15 insertions, 19 deletions
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index a2486566af..226234d386 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -992,6 +992,14 @@ static int local_unlinkat_common(FsContext *ctx, int dirfd, const char *name,
if (ctx->export_flags & V9FS_SM_MAPPED_FILE) {
int map_dirfd;
+ /* We need to remove the metadata as well:
+ * - the metadata directory if we're removing a directory
+ * - the metadata file in the parent's metadata directory
+ *
+ * If any of these are missing (ie, ENOENT) then we're probably
+ * trying to remove something that wasn't created in mapped-file
+ * mode. We just ignore the error.
+ */
if (flags == AT_REMOVEDIR) {
int fd;
@@ -999,32 +1007,20 @@ static int local_unlinkat_common(FsContext *ctx, int dirfd, const char *name,
if (fd == -1) {
goto err_out;
}
- /*
- * If directory remove .virtfs_metadata contained in the
- * directory
- */
ret = unlinkat(fd, VIRTFS_META_DIR, AT_REMOVEDIR);
close_preserve_errno(fd);
if (ret < 0 && errno != ENOENT) {
- /*
- * We didn't had the .virtfs_metadata file. May be file created
- * in non-mapped mode ?. Ignore ENOENT.
- */
goto err_out;
}
}
- /*
- * Now remove the name from parent directory
- * .virtfs_metadata directory.
- */
map_dirfd = openat_dir(dirfd, VIRTFS_META_DIR);
- ret = unlinkat(map_dirfd, name, 0);
- close_preserve_errno(map_dirfd);
- if (ret < 0 && errno != ENOENT) {
- /*
- * We didn't had the .virtfs_metadata file. May be file created
- * in non-mapped mode ?. Ignore ENOENT.
- */
+ if (map_dirfd != -1) {
+ ret = unlinkat(map_dirfd, name, 0);
+ close_preserve_errno(map_dirfd);
+ if (ret < 0 && errno != ENOENT) {
+ goto err_out;
+ }
+ } else if (errno != ENOENT) {
goto err_out;
}
}