Rsync cannot copy to a vfat partition on kernel 2.6.25

Rsync cannot copy to a vfat partition on kernel 2.6.25

[Dave Jones]

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.

	Dave

-- 
http://www.codemonkey.org.uk

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

[OGAWA Hirofumi]

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;
_

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

[Pavel Machek]

Hi!

> > 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.

Hmm... but I guess mkstemp is no longer safe with this?

So we have choice between security hole and regression...?

							Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

[OGAWA Hirofumi]

Pavel Machek <pavel@ucw.cz> writes:

> Hi!

Hi,

>> > 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.
>
> Hmm... but I guess mkstemp is no longer safe with this?
>
> So we have choice between security hole and regression...?

Maybe. But if users choose the group or world writable umask, I guess
nobody would care the permission of temporary file, because all file is
writable always. Um..
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

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

[OGAWA Hirofumi]

OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> writes:

> Maybe. But if users choose the group or world writable umask, I guess
                                                         ^^^^^
                                                         umask mount option
> nobody would care the permission of temporary file, because all file is
> writable always. Um..
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

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

[OGAWA Hirofumi]

OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> 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.
>>
>> Hmm... but I guess mkstemp is no longer safe with this?
>>
>> So we have choice between security hole and regression...?
>
> Maybe. But if users choose the group or world writable umask, I guess
> nobody would care the permission of temporary file, because all file is
> writable always. Um..

BTW, if users specified "quiet" option, FAT driver will ignore some
permission check (uid, gid, etc.).

So, another solution would be to specify this option, or change default
of it.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

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

[OGAWA Hirofumi]

OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> writes:

> BTW, if users specified "quiet" option, FAT driver will ignore some
> permission check (uid, gid, etc.).

And it will return 0 without changing attribute.

> So, another solution would be to specify this option, or change default
> of it.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

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

[Pavel Machek]

On Mon 2008-06-02 06:12:23, OGAWA Hirofumi wrote:
> Pavel Machek <pavel@ucw.cz> writes:
> 
> > Hi!
> 
> Hi,
> 
> >> > 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.
> >
> > Hmm... but I guess mkstemp is no longer safe with this?
> >
> > So we have choice between security hole and regression...?
> 
> Maybe. But if users choose the group or world writable umask, I guess
> nobody would care the permission of temporary file, because all file is
> writable always. Um..

Okay, if the user wants his vfat world-readable, it is hard to create
security hole there.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html