Skip to content

Commit

Permalink
qemu_test: Rebase force-uid0-on-9p.patch
Browse files Browse the repository at this point in the history 
This reverts commit 3a4e237.

The reverted commit caused the fix for CVE-2016-9602 not to be applied
for qemu_test because it conflicts with the force-uid0-on-9p.patch.

So with the rebase of the patch on top of the changes of the
CVE-2016-9602.patch, both patches no longer conflict with each other.

I've tested this with the "misc" NixOS test and it succeeds.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
  • Loading branch information
@aszlig
aszlig committed Mar 11, 2017
1 parent c3c6953 commit 0a7673d
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 40 deletions.
14 changes: 7 additions & 7 deletions pkgs/applications/virtualization/qemu/default.nix
View file
Expand Up @@ -77,6 +77,12 @@ stdenv.mkDerivation rec {
"0lk19qss6ky7cqnvis54742cr2z0vl8c64chhch0kp6n83hray9x")

# security fixes from debian
(fetchurl {
name = "CVE-2016-9602.patch";
url = "https://anonscm.debian.org/cgit/pkg-qemu/qemu.git/plain/debian/patches/9pfs-symlink-attack-fixes-CVE-2016-9602.patch?h=debian/qemu_2.8%2bdfsg-3";
sha256 = "0f7m1k3hbw9v0dwqn53ds36s7s334vlidvbn0682s9r2sq0sjlkv";
})

(fetchurl {
name = "CVE-2017-2630.patch";
url = "https://anonscm.debian.org/cgit/pkg-qemu/qemu.git/plain/debian/patches/nbd_client-fix-drop_sync-CVE-2017-2630.patch?h=debian/qemu_2.8%2bdfsg-3";
Expand Down Expand Up @@ -135,13 +141,7 @@ stdenv.mkDerivation rec {
(upstreamPatch "CVE-2017-5987" "6e86d90352adf6cb08295255220295cf23c4286e"
"09yfxf93cisx8rhm0h48ib1ibwfs420k5pqpz8dnz33nci9567jm")

] ++ (if nixosTestRunner then [ ./force-uid0-on-9p.patch ] else [
(fetchurl {
name = "CVE-2016-9602.patch";
url = "https://anonscm.debian.org/cgit/pkg-qemu/qemu.git/plain/debian/patches/9pfs-symlink-attack-fixes-CVE-2016-9602.patch?h=debian/qemu_2.8%2bdfsg-3";
sha256 = "0f7m1k3hbw9v0dwqn53ds36s7s334vlidvbn0682s9r2sq0sjlkv";
})
]);
] ++ optional nixosTestRunner ./force-uid0-on-9p.patch;

hardeningDisable = [ "stackprotector" ];

Expand Down
70 changes: 37 additions & 33 deletions pkgs/applications/virtualization/qemu/force-uid0-on-9p.patch
View file
@@ -1,8 +1,8 @@
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 3f271fc..dc273f4 100644
index 45e9a1f9b0..494ee00c66 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -45,6 +45,23 @@
@@ -84,6 +84,23 @@ static void unlinkat_preserve_errno(int dirfd, const char *path, int flags)

#define VIRTFS_META_DIR ".virtfs_metadata"

Expand All @@ -23,10 +23,10 @@ index 3f271fc..dc273f4 100644
+ return 0;
+}
+
static char *local_mapped_attr_path(FsContext *ctx, const char *path)
static FILE *local_fopenat(int dirfd, const char *name, const char *mode)
{
int dirlen;
@@ -128,6 +145,8 @@ static int local_lstat(FsContext *fs_ctx, V9fsPath *fs_path, struct stat *stbuf)
int fd, o_mode = 0;
@@ -161,6 +178,8 @@ static int local_lstat(FsContext *fs_ctx, V9fsPath *fs_path, struct stat *stbuf)
if (err) {
goto err_out;
}
Expand All @@ -35,28 +35,17 @@ index 3f271fc..dc273f4 100644
if (fs_ctx->export_flags & V9FS_SM_MAPPED) {
/* Actual credentials are part of extended attrs */
uid_t tmp_uid;
@@ -462,6 +481,11 @@ static ssize_t local_pwritev(FsContext *ctx, V9fsFidOpenState *fs,
return ret;
}
@@ -280,6 +299,9 @@ static int fchmodat_nofollow(int dirfd, const char *name, mode_t mode)
{
int fd, ret;

+static inline int maybe_chmod(const char *path, mode_t mode)
+{
+ return is_in_store_path(path) ? 0 : chmod(path, mode);
+}
+ if (is_in_store_path(name))
+ return 0;
+
static int local_chmod(FsContext *fs_ctx, V9fsPath *fs_path, FsCred *credp)
{
char *buffer;
@@ -477,7 +501,7 @@ static int local_chmod(FsContext *fs_ctx, V9fsPath *fs_path, FsCred *credp)
} else if ((fs_ctx->export_flags & V9FS_SM_PASSTHROUGH) ||
(fs_ctx->export_flags & V9FS_SM_NONE)) {
buffer = rpath(fs_ctx, path);
- ret = chmod(buffer, credp->fc_mode);
+ ret = maybe_chmod(buffer, credp->fc_mode);
g_free(buffer);
}
return ret;
@@ -621,6 +645,8 @@ static int local_fstat(FsContext *fs_ctx, int fid_type,
/* FIXME: this should be handled with fchmodat(AT_SYMLINK_NOFOLLOW).
* Unfortunately, the linux kernel doesn't implement it yet. As an
* alternative, let's open the file and use fchmod() instead. This
@@ -661,6 +683,8 @@ static int local_fstat(FsContext *fs_ctx, int fid_type,
if (err) {
return err;
}
Expand All @@ -65,13 +54,28 @@ index 3f271fc..dc273f4 100644
if (fs_ctx->export_flags & V9FS_SM_MAPPED) {
/* Actual credentials are part of extended attrs */
uid_t tmp_uid;
@@ -916,7 +942,8 @@ static int local_chown(FsContext *fs_ctx, V9fsPath *fs_path, FsCred *credp)
@@ -795,8 +819,11 @@ static int local_symlink(FsContext *fs_ctx, const char *oldpath,
if (err) {
goto out;
}
- err = fchownat(dirfd, name, credp->fc_uid, credp->fc_gid,
- AT_SYMLINK_NOFOLLOW);
+ if (is_in_store_path(name))
+ err = 0;
+ else
+ err = fchownat(dirfd, name, credp->fc_uid, credp->fc_gid,
+ AT_SYMLINK_NOFOLLOW);
if (err == -1) {
/*
* If we fail to change ownership and if we are
@@ -911,7 +938,9 @@ static int local_chown(FsContext *fs_ctx, V9fsPath *fs_path, FsCred *credp)
goto out;
}

- if ((credp->fc_uid == -1 && credp->fc_gid == -1) ||
+ if (is_in_store_path(name)) {
+ ret = 0;
+ } else if ((credp->fc_uid == -1 && credp->fc_gid == -1) ||
(fs_ctx->export_flags & V9FS_SM_PASSTHROUGH) ||
(fs_ctx->export_flags & V9FS_SM_NONE)) {
buffer = rpath(fs_ctx, path);
- ret = lchown(buffer, credp->fc_uid, credp->fc_gid);
+ ret = is_in_store_path(buffer)
+ ? 0 : lchown(buffer, credp->fc_uid, credp->fc_gid);
g_free(buffer);
} else if (fs_ctx->export_flags & V9FS_SM_MAPPED) {
buffer = rpath(fs_ctx, path);
ret = fchownat(dirfd, name, credp->fc_uid, credp->fc_gid,

2 comments on commit 0a7673d

@aszlig
Copy link
Member Author

@aszlig aszlig commented on 0a7673d Mar 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fpletz: I've only pushed this to master because I have only tested this against the misc VM test. So I'd suggest to wait for the unstable tests before cherry-picking this to stable until we know that this doesn't cause regressions.

@fpletz
Copy link
Member

@fpletz fpletz commented on 0a7673d Mar 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense.Thanks a lot!

Please sign in to comment.