Re: Rsync cannot copy to a vfat partition on kernel 2.6.25

[OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>]

Dave Jones <davej@redhat.com> writes:

> We had a user report at https://bugzilla.redhat.com/show_bug.cgi?id=449080
> that in 2.6.25, he can no longer rsync to a vfat partition, even as root.
> I just reproduced this here. It gets -EPERM in the mkstemp call.
> (full strace in the bug report).
>
> Did we change behaviour somehow in the vfat code?
> 2.6.24.7 works fine apparently.

Yes, it was changed. New one allows only acceptable chmod(), and if not
acceptable, it returns -EPERM. Old one allows even if it can't store the
disk inode. But it may be too strict for users.

Umm.. anyway, the following patch (still untested) will relax the check...
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>




Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---

 fs/fat/file.c |   25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff -puN fs/fat/file.c~fat_setattr-fix fs/fat/file.c
--- linux-2.6/fs/fat/file.c~fat_setattr-fix	2008-05-31 03:49:09.000000000 +0900
+++ linux-2.6-hirofumi/fs/fat/file.c	2008-05-31 04:03:45.000000000 +0900
@@ -262,7 +262,7 @@ static int fat_check_mode(const struct m
 {
 	mode_t mask, req = mode & ~S_IFMT;
 
-	if (S_ISREG(mode))
+	if (S_ISREG(inode->i_mode))
 		mask = sbi->options.fs_fmask;
 	else
 		mask = sbi->options.fs_dmask;
@@ -299,7 +299,7 @@ int fat_setattr(struct dentry *dentry, s
 {
 	struct msdos_sb_info *sbi = MSDOS_SB(dentry->d_sb);
 	struct inode *inode = dentry->d_inode;
-	int mask, error = 0;
+	int error = 0;
 	unsigned int ia_valid;
 
 	lock_kernel();
@@ -332,12 +332,13 @@ int fat_setattr(struct dentry *dentry, s
 			error = 0;
 		goto out;
 	}
+
 	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) &&
-	     fat_check_mode(sbi, inode, attr->ia_mode) < 0))
+	     (attr->ia_mode & ~MSDOS_VALID_MODE)))
 		error = -EPERM;
 
 	if (error) {
@@ -346,15 +347,17 @@ int fat_setattr(struct dentry *dentry, s
 		goto out;
 	}
 
-	error = inode_setattr(inode, attr);
-	if (error)
-		goto out;
+	/*
+	 * If we can't store to storage, don't set this permission.
+	 * We don't return -EPERM here. Yes, strange, but this is too
+	 * old behavior.
+	 */
+	if (attr->ia_valid & ATTR_MODE) {
+		if (fat_check_mode(sbi, inode, attr->ia_mode) < 0)
+			attr->ia_mode &= ~ATTR_MODE;
+	}
 
-	if (S_ISDIR(inode->i_mode))
-		mask = sbi->options.fs_dmask;
-	else
-		mask = sbi->options.fs_fmask;
-	inode->i_mode &= S_IFMT | (S_IRWXUGO & ~mask);
+	error = inode_setattr(inode, attr);
 out:
 	unlock_kernel();
 	return error;
_