Message ID | 4954.1432838811@warthog.procyon.org.uk (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Hi Miklós, Al, Any thoughts on taking this upstream? David -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 28, 2015, at 02:46 PM, David Howells wrote: > Print a warning when overlayfs copies up a file if the process that triggered > the copy up has a R/O fd open to the lower file being copied up. Why not an option to make it an error? If one's goal is to use overlayfs without apps potentially corrupting data, better to fail fast. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Colin Walters <walters@verbum.org> wrote: > On Thu, May 28, 2015, at 02:46 PM, David Howells wrote: > > Print a warning when overlayfs copies up a file if the process that > > triggered the copy up has a R/O fd open to the lower file being copied up. > > Why not an option to make it an error? If one's goal is to use overlayfs > without apps potentially corrupting data, better to fail fast. That could be added. Note that doing this isn't *necessarily* wrong though. David -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Al, > Print a warning when overlayfs copies up a file if the process that triggered > the copy up has a R/O fd open to the lower file being copied up. Can you take this? David -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c index 24f640441bd9..04f6f75ba4a9 100644 --- a/fs/overlayfs/copy_up.c +++ b/fs/overlayfs/copy_up.c @@ -7,6 +7,7 @@ * the Free Software Foundation. */ +#include <linux/module.h> #include <linux/fs.h> #include <linux/slab.h> #include <linux/file.h> @@ -16,10 +17,40 @@ #include <linux/uaccess.h> #include <linux/sched.h> #include <linux/namei.h> +#include <linux/fdtable.h> #include "overlayfs.h" #define OVL_COPY_UP_CHUNK_SIZE (1 << 20) +static bool __read_mostly ovl_check_copy_up; +module_param_named(check_copy_up, ovl_check_copy_up, bool, + S_IWUSR | S_IRUGO); +MODULE_PARM_DESC(ovl_check_copy_up, + "Warn on copy-up when causing process also has a R/O fd open"); + +static int ovl_check_fd(const void *data, struct file *f, unsigned fd) +{ + const struct dentry *dentry = data; + + if (f->f_path.dentry == dentry) + pr_warn("overlayfs: Warning: Copying up %pD, but open R/O on fd %u which will cease to be coherent [pid=%d %s]\n", + f, fd, current->pid, current->comm); + return 0; +} + +/* + * Check the fds open by this process and warn if something like the following + * scenario is about to occur: + * + * fd1 = open("foo", O_RDONLY); + * fd2 = open("foo", O_RDWR); + */ +static void ovl_do_check_copy_up(struct dentry *dentry) +{ + if (ovl_check_copy_up) + iterate_fd(current->files, 0, ovl_check_fd, dentry); +} + int ovl_copy_xattr(struct dentry *old, struct dentry *new) { ssize_t list_size, size; @@ -299,6 +330,8 @@ int ovl_copy_up_one(struct dentry *parent, struct dentry *dentry, struct cred *override_cred; char *link = NULL; + ovl_do_check_copy_up(lowerpath->dentry); + ovl_path_upper(parent, &parentpath); upperdir = parentpath.dentry;
Print a warning when overlayfs copies up a file if the process that triggered the copy up has a R/O fd open to the lower file being copied up. This can help catch applications that do things like the following: fd1 = open("foo", O_RDONLY); fd2 = open("foo", O_RDWR); where they expect fd1 and fd2 to refer to the same file - which will no longer be the case post-copy up. With this patch, the following commands: bash 5</mnt/a/foo128 6<>/mnt/a/foo128 assuming /mnt/a/foo128 to be an un-copied up file on an overlay will produce the following warning in the kernel log: overlayfs: Copying up foo129, but open R/O on fd 5 which will cease to be coherent [pid=3818 bash] This is enabled by setting: /sys/module/overlay/parameters/check_copy_up to 1. Signed-off-by: David Howells <dhowells@redhat.com> --- -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html