Martin,
Thanks for the kind words about Unionfs. We plan on having a release
just as soon was we get the regression test working (there is a nasty
refcount bug involving rename of directories).
This patch looks good, but I have one minor suggestion. Instead of
making this a mount-wide option, how about a per-branch option. Instead
of something like:
mount -t unionfs -o dirs=/b0:/b1=ro,nfsperms=insecure none /mnt/unionfs
How about:
mount -t unionfs -o dirs=/b0:/b1=nfsro none /mnt/unionfs
Jeff, in the interim, please apply this patch.
Charles
On Fri, 2006-01-13 at 11:43 +0100, Martin Kreiner wrote:
> hi,
>
> as a start i am using unionfs for about half a year and the current CVS
> versions are the most stable ever. no oops for months in my nfs-root
> related environment. well done!
> so how about a new release (1.1.2) ;-)?
>
> as i am working with nfs-mounted branches i have to compile unionfs.ko
> with "-DNFS_SECURITY_HOLE" to get rid of some "-EACCES" return values of
> my nfs server. it would be handy to set an equivalent mount option for
> any union that is used. this would lead to the following advantages:
>
> - it would be possible to have independent secure and insecure mounts
> side by side
>
> - upstream kernels/modules could be used in nfs related environments
> without the need for recompiling or a global security hole
>
> beneath is a patch providing a per union mount option "nfsperms". this
> option can be "secure" (default) or "insecure". so if you mount your
> union with "nfsperms=insecure" the nfs server return value "-EACCES"
> leads to a standard Unix permissions check only in this union.
>
> cheers,
> martin
>
> --- unionfs-20060112-2205/unionfs.h.org 2006-01-13 09:01:26.418630983
> +0000
> +++ unionfs-20060112-2205/unionfs.h 2006-01-13 09:29:52.058797978 +0000
> @@ -750,7 +750,10 @@
> /* 1 = f/s mounter permission, 0 = check for COPYUP_OWNER */
> #define COPYUP_FS_MOUNTER 16
>
> -#define VALID_MOUNT_FLAGS (DELETE_WHITEOUT | COPYUP_OWNER |
> COPYUP_FS_MOUNTER)
> +/* 1 = only standard Unix permissions check on nfs if -EACCESS, 0 = no
> special permission treatment */
> +#define NFS_INSECURE 32
> +
> +#define VALID_MOUNT_FLAGS (DELETE_WHITEOUT | COPYUP_OWNER |
> COPYUP_FS_MOUNTER | NFS_INSECURE)
>
> /*
> * MACROS:
> --- unionfs-20060112-2205/main.c.org 2006-01-13 09:33:34.857822350 +0000
> +++ unionfs-20060112-2205/main.c 2006-01-13 09:44:12.964777406 +0000
> @@ -523,6 +523,20 @@
> }
> continue;
> }
> + if (!strcmp("nfsperms", optname)) {
> + if (!strcmp("secure", optarg)) {
> + /* default */
> + } else if (!strcmp("insecure", optarg)) {
> + MOUNT_FLAG(sb) |= NFS_INSECURE;
> + } else {
> + printk(KERN_WARNING
> + "unionfs: invalid nfsperms option
> '%s'\n",
> + optarg);
> + err = -EINVAL;
> + goto out_error;
> + }
> + continue;
> + }
>
> /* All of these options require an integer argument. */
> intval = simple_strtoul(optarg, &endptr, 0);
> --- unionfs-20060112-2205/super.c.org 2006-01-13 09:45:01.187229205 +0000
> +++ unionfs-20060112-2205/super.c 2006-01-13 09:47:25.073787777 +0000
> @@ -472,6 +472,12 @@
> seq_printf(m, ",copyup=mounter");
> else
> seq_printf(m, ",copyup=preserve");
> +
> + if (IS_SET(sb, NFS_INSECURE))
> + seq_printf(m, ",nfsperms=insecure");
> + else
> + seq_printf(m, ",nfsperms=secure");
> +
> out:
> if (tmp)
> free_page(tmp);
> --- unionfs-20060112-2205/inode.c.org 2006-01-13 09:47:38.471301544 +0000
> +++ unionfs-20060112-2205/inode.c 2006-01-13 09:55:40.934757358 +0000
> @@ -806,10 +806,11 @@
> }
>
> /* Basically copied from the kernel vfs permission(), but we've changed
> - * the following: (1) the IS_RDONLY check is skipped, and (2) if you define
> - * -DNFS_SECURITY_HOLE, we assume that -EACCES means that the export is
> - * read-only and we should check standard Unix permissions. This means
> - * that NFS ACL checks (or other advanced permission features) are bypassed.
> + * the following: (1) the IS_RDONLY check is skipped, and (2) if you set
> + * the mount option `nfsperms=insceure', we assume that -EACCES means that
> + * the export is read-only and we should check standard Unix permissions.
> + * This means that NFS ACL checks (or other advanced permission features)
> + * are bypassed.
> */
> static int inode_permission(struct inode *inode, int mask, struct nameidata
> *nd,
> int bindex)
> @@ -835,17 +836,15 @@
> submask = mask & ~MAY_APPEND;
> if (inode->i_op && inode->i_op->permission) {
> retval = inode->i_op->permission(inode, submask, nd);
> -#ifdef NFS_SECURITY_HOLE
> -#define IS_NFS(inode) (!strcmp("nfs", (inode)->i_sb->s_type->name))
> if ((retval == -EACCES) && (submask & MAY_WRITE) &&
> - IS_NFS(inode)) {
> + (!strcmp("nfs", (inode)->i_sb->s_type->name)) &&
> + IS_SET(nd->mnt->mnt_sb, NFS_INSECURE)) {
> #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,10)
> retval = vfs_permission(inode, submask);
> #else
> retval = generic_permission(inode, submask, NULL);
> #endif
> }
> -#endif
> } else {
> #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,10)
> retval = vfs_permission(inode, submask);
> _______________________________________________
> unionfs mailing list
> [email protected]
> http://www.fsl.cs.sunysb.edu/mailman/listinfo/unionfs
_______________________________________________
unionfs mailing list
[email protected]
http://www.fsl.cs.sunysb.edu/mailman/listinfo/unionfs
