lkml.org 
[lkml]   [2007]   [Aug]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [patch] Refine FAT chmod checks
From
Date
Jan Engelhardt <jengelh@computergmbh.de> writes:

> when a vfat filesystem is mounted without the quiet option, chown fails,
> but chmod still succeeds. I think that is wrong.

Could you explain why this is wrong more?

Thanks.

> [fs/fat/]: Refine FAT chmod checks
>
> Prohibit mode changes in non-quiet mode that cannot be stored reliably
> with the on-disk format.
>
> Signed-off-by: Jan Engelhardt <jengelh@gmx.de>
>
> ---
> fs/fat/file.c | 47 +++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 43 insertions(+), 4 deletions(-)
>
> Index: linux-2.6.22/fs/fat/file.c
> ===================================================================
> --- linux-2.6.22.orig/fs/fat/file.c
> +++ linux-2.6.22/fs/fat/file.c
> @@ -155,6 +155,42 @@ out:
> return err;
> }
>
> +static int check_mode(const struct msdos_sb_info *sbi, mode_t mode)
> +{
> + mode_t req = mode & ~S_IFMT;
> +
> + /*
> + * Of the r and x bits, all (subject to umask) must be present. Of the
> + * w bits, either all (subject to umask) or none must be present.
> + */
> +
> + if (S_ISREG(mode)) {
> + req &= ~sbi->options.fs_fmask;
> +
> + if ((req & (S_IRUGO | S_IXUGO)) !=
> + ((S_IRUGO | S_IXUGO) & ~sbi->options.fs_fmask))
> + return -EPERM;
> +
> + if ((req & S_IWUGO) != 0 &&
> + (req & S_IWUGO) != (S_IWUGO & ~sbi->options.fs_fmask))
> + return -EPERM;
> + } else if (S_ISDIR(mode)) {
> + req &= ~sbi->options.fs_dmask;
> +
> + if ((req & (S_IRUGO | S_IXUGO)) !=
> + ((S_IRUGO | S_IXUGO) & ~sbi->options.fs_dmask))
> + return -EPERM;
> +
> + if ((req & S_IWUGO) != 0 &&
> + (req & S_IWUGO) != (S_IWUGO & ~sbi->options.fs_dmask))
> + return -EPERM;
> + } else {
> + return -EPERM;
> + }
> +
> + return 0;
> +}
> +
> int fat_notify_change(struct dentry *dentry, struct iattr *attr)
> {
> struct msdos_sb_info *sbi = MSDOS_SB(dentry->d_sb);
> @@ -186,16 +222,19 @@ int fat_notify_change(struct dentry *den
> if (((attr->ia_valid & ATTR_UID) &&
> (attr->ia_uid != sbi->options.fs_uid)) ||
> ((attr->ia_valid & ATTR_GID) &&
> - (attr->ia_gid != sbi->options.fs_gid)) ||
> - ((attr->ia_valid & ATTR_MODE) &&
> - (attr->ia_mode & ~MSDOS_VALID_MODE)))
> + (attr->ia_gid != sbi->options.fs_gid)))
> error = -EPERM;
> -
> if (error) {
> if (sbi->options.quiet)
> error = 0;
> goto out;
> }
> +
> + if (error == 0 && (attr->ia_valid & ATTR_MODE))
> + if ((error = check_mode(sbi, attr->ia_mode)) != 0 &&
> + sbi->options.quiet)
> + error = 0;

This test is really here? error is always "0", and anybody doesn't
check after that.

> error = inode_setattr(inode, attr);
> if (error)
> goto out;

--
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2007-08-20 17:19    [W:0.042 / U:4.284 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site