Re: [PATCH 04/13] overlay filesystem

ebiederm@xmission.com (Eric W. Biederman) writes:

Miklos Szeredi [off-list ref] writes:

Minor nits below.

quoted

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
new file mode 100644
index 0000000..6b50823
--- /dev/null
+++ b/fs/overlayfs/dir.c

@@ -0,0 +1,598 @@
+/*
+ *
+ * Copyright (C) 2011 Novell Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+
+#include <linux/fs.h>
+#include <linux/namei.h>
+#include <linux/xattr.h>
+#include <linux/security.h>
+#include <linux/cred.h>
+#include "overlayfs.h"
+
+static const char *ovl_whiteout_symlink = "(overlay-whiteout)";
+
+static int ovl_whiteout(struct dentry *upperdir, struct dentry *dentry)
+{
+	int err;
+	struct dentry *newdentry;
+	const struct cred *old_cred;
+	struct cred *override_cred;
+
+	/* FIXME: recheck lower dentry to see if whiteout is really
 	needed */

Is that FIXME still valid?

It is, but it's not an important feature.  Lacking this will mean once a
file/directory is marked whiteout or opaque on the upper filesystem it
will remain so forever even after the file/directory it is masking out
has been removed from the lower filesystem.

However this cannot be observed by looking at the overlay, only by
looking at the underlying filesystems.

quoted

+	err = -ENOMEM;
+	override_cred = prepare_creds();
+	if (!override_cred)
+		goto out;
+
+	/*
+	 * CAP_SYS_ADMIN for setxattr
+	 * CAP_DAC_OVERRIDE for symlink creation
+	 * CAP_FOWNER for unlink in sticky directory
+	 */
+	cap_raise(override_cred->cap_effective, CAP_SYS_ADMIN);
+	cap_raise(override_cred->cap_effective, CAP_DAC_OVERRIDE);
+	cap_raise(override_cred->cap_effective, CAP_FOWNER);
+	override_cred->fsuid = 0;
+	override_cred->fsgid = 0;

Could you please make these GLOBAL_ROOT_UID and GLOBAL_ROOT_GID
instead of 0?  Otherwise this code won't compile with the usernamespace
bits enabled.

Okay.

Thanks for the review.

Miklos