Bug 122047 - [ext2fs] [patch] incorrect handling of UF_IMMUTABLE / UF_APPEND flag on EXT2FS (maybe others)
Summary: [ext2fs] [patch] incorrect handling of UF_IMMUTABLE / UF_APPEND flag on EXT2F...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 6.3-STABLE
Hardware: Any Any
: Normal Affects Only Me
Assignee: Jaakko Heinonen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-03-24 13:10 UTC by Ighighi
Modified: 2010-04-07 16:52 UTC (History)
0 users

See Also:


Attachments
ext2fs.patch.txt (2.87 KB, text/plain)
2008-05-29 07:46 UTC, Ighighi
no flags Details
ext2fs.patch.txt (2.87 KB, text/plain)
2008-05-29 08:36 UTC, Ighighi
no flags Details
ext2fs.patch.txt (4.61 KB, text/plain)
2008-06-02 06:35 UTC, Ighighi
no flags Details
uf.patch (2.15 KB, patch)
2009-05-29 11:27 UTC, aditya sarawgi
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ighighi 2008-03-24 13:10:02 UTC
On EXT2 filesystems (and maybe others), the kernels seems to handle
UF_IMMUTABLE and SF_IMMUTABLE separately only until they're unmounted.
When these filesystems are mounted afterwards, both flags are set. This
of course happens because EXT2FS has only one immutable bit.

So it is now a question of policy:

How should we map UF_IMMUTABLE & SF_IMMUTABLE to EXT2_IMMUTABLE_FL ?

The problem is that users may set any of the UF_* flags.  If any of these is 
later made SF_ too, those UF_* flags would be useless (and inconvenient) for
the user because suddenly they can't turn them off and work on files they own...
So being those flags usable by root alone, we must ask this question:
Are they useful to the superuser or they are not ?  If they are not, then 
dropping support for append/immutable flags for EXT2FS makes sense. If they are,
we shouldn't allow normal users to set those SF_* flags in this indirect way.

In this case, the issue of policy is better left on the administrator to choose
from, as it's done in the UNIX world ;)

I found this kernel option in NetBSD in options(5):
     options EXT2FS_SYSTEM_FLAGS
     This option changes the behavior of the APPEND and IMMUTABLE flags for a
     file on an EXT2FS file system.  Without this option, the superuser or
     owner of the file can set and clear them.	With this option, only the
     superuser can set them, and they can't be cleared if the securelevel is
     greater than 0.  See also chflags(1).

I believe it'd be best on a per-filesystem basis as a mount() option anyway and
to ignore the SF_* flags on filesystems mounted by normal users by default.

FWIW, I think this PR may apply to REISERFS and DragonFly.

How-To-Repeat: As root, run:

# /bin/dd if=/dev/zero of=/tmp/ext2fs.img bs=1k count=1024
1024+0 records in
1024+0 records out
1048576 bytes transferred in 0.042202 secs (24846597 bytes/sec)                      
# device=$(/sbin/mdconfig -f /tmp/ext2fs.img)
# /usr/local/sbin/mke2fs /dev/$device
# /bin/mkdir /tmp/ext2fs.mnt
# /sbin/mount_ext2fs /dev/$device /tmp/ext2fs.mnt
# /bin/mkdir -m 1777 /tmp/ext2fs.mnt/tmp                                      

Then, on a user account:

$ touch /tmp/ext2fs.mnt/tmp/foobar
$ /bin/ls -lo /tmp/ext2fs.mnt/tmp/foobar
-rw-r--r--  1 ighighi  wheel  - 0 Mar 24 06:57 /tmp/ext2fs.mnt/tmp/foobar            
$ /bin/chflags uchg /tmp/ext2fs.mnt/tmp/foobar
$ /bin/ls -lo /tmp/ext2fs.mnt/tmp/foobar
-rw-r--r--  1 ighighi  wheel  uchg 0 Mar 24 06:57 /tmp/ext2fs.mnt/tmp/foobar
$ /bin/chflags nouchg /tmp/ext2fs.mnt/tmp/foobar
$ ls -l /tmp/ext2fs.mnt/tmp/foobar
-rw-r--r--  1 ighighi  wheel  - 0 Mar 24 06:57 /tmp/ext2fs.mnt/tmp/foobar            
$ /bin/chflags uchg /tmp/ext2fs.mnt/tmp/foobar

Then, as root, run:

# /sbin/umount -v /tmp/ext2fs.mnt
/dev/md0: unmount from /var/tmp/ext2fs.mnt                                           
# /sbin/mount_ext2fs /dev/$device /tmp/ext2fs.mnt                                                 

Now, as the user, run:

$ /bin/ls -lo /tmp/ext2fs.mnt/tmp/foobar
-rw-r--r--  1 ighighi  wheel  schg,uchg 0 Mar 24 06:57 tmp/ext2fs.mnt/tmp/foobar    
$ /bin/chflags nouchg /tmp/ext2fs.mnt/tmp/foobar
chflags: /tmp/ext2fs.mnt/tmp/foobar: Operation not permitted
Comment 1 Ighighi 2008-05-29 07:30:09 UTC
See attached patch.
Comment 2 Ighighi 2008-05-29 07:46:35 UTC
See attached patch.
Comment 3 Ighighi 2008-05-29 08:36:30 UTC
See attached patch.  Gmail sucks at sending patches to GNATS =(
Comment 4 Ighighi 2008-06-02 06:35:03 UTC
On Linux, only the root user may set/clear the immutable/append flags
on ext2 filesystems... Shouldn't FreeBSD do this too, as a POLA?

Anyway the attached patch extends the previous one by making it possible
to follow the current Linux convention by setting the sysctl to 0.
Setting it to 1, allows normal users to set them as well, and setting it
to -1 preserves current (though erroneous) FreeBSD behavior.
Comment 5 Julian Elischer 2008-06-02 08:14:46 UTC
Ighighi wrote:
> On Linux, only the root user may set/clear the immutable/append flags
> on ext2 filesystems... Shouldn't FreeBSD do this too, as a POLA?

No I think it should preserver the BSD scheme where being able to
change the immutable bits is controlled by the system secure level.
(and your UID of course). At least I think that is what I would
expect. (All file systems to behave about the same for a
particular OS.


> 
> Anyway the attached patch extends the previous one by making it possible
> to follow the current Linux convention by setting the sysctl to 0.
> Setting it to 1, allows normal users to set them as well, and setting it
> to -1 preserves current (though erroneous) FreeBSD behavior.


> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> freebsd-fs@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-fs
> To unsubscribe, send any mail to "freebsd-fs-unsubscribe@freebsd.org"
Comment 6 Bruce Evans freebsd_committer freebsd_triage 2008-06-02 08:50:45 UTC
On Mon, 2 Jun 2008, Ighighi wrote:

> On Linux, only the root user may set/clear the immutable/append flags
> on ext2 filesystems... Shouldn't FreeBSD do this too, as a POLA?

I think it should do just that...

> Anyway the attached patch extends the previous one by making it possible
> to follow the current Linux convention by setting the sysctl to 0.
> Setting it to 1, allows normal users to set them as well, and setting it
> to -1 preserves current (though erroneous) FreeBSD behavior.

... but nothing more.  Why have complications provide more control over
Linux file systems than Linux does?  The current behaviour seems to be
just a bug from not understanding that Linux has no user immutable/append
flags.

% --- src/sys/gnu/fs/ext2fs/ext2_inode_cnv.c.orig	2005-06-14 22:36:10.000000000 -0400
% +++ src/sys/gnu/fs/ext2fs/ext2_inode_cnv.c	2008-06-02 00:35:34.658524358 -0430
% @@ -30,11 +30,19 @@
%  #include <sys/lock.h>
%  #include <sys/stat.h>
%  #include <sys/vnode.h>
% +#include <sys/kernel.h>
% +#include <sys/sysctl.h>
% 
%  #include <gnu/fs/ext2fs/inode.h>
%  #include <gnu/fs/ext2fs/ext2_fs.h>
%  #include <gnu/fs/ext2fs/ext2_extern.h>
% 
% +SYSCTL_DECL(_vfs_e2fs);
% +
% +static int userflags = -1;
% +SYSCTL_INT(_vfs_e2fs, OID_AUTO, userflags, CTLFLAG_RW,
% +    &userflags, 0, "Users may set/clear filesystem flags");
% +
%  void
%  ext2_print_inode( in )
%  	struct inode *in;

The existence of vfs sysctls is another bug.  They should be mount options,
or perhaps system-wide, or not exist at all.  ext2fs has only one vfs sysctl
now:
- vfs.e2fs.dircheck.  This sysctl is less broken than in ffs, where the
   corresponding sysctl is spelled debug.dircheck and a comment still
   says that the corresponding static kernel variable `dirchk' is changed
   by "patching".  The kernel variable is spelled differently to the
   sysctl to confuse me when grepping for dircheck.
- the debug.doasyncfree sysctl is in dead code (under the non-option
   FANCY_REALLOC.  Block reallocation is also dead in ext2fs).  This
   sysctl is less broken in ffs.  There it is named vfs.ffs.doasyncfree.
OTOH, perhaps these sysctls really did belong under debug or vfs.debug.
It is not very useful to restrict them to just ffs or ext2fs when 
have many mounted file systems.

This bug should not be extended.

% @@ -83,8 +91,37 @@ ext2_ei2i(ei, ip)
%  	ip->i_mtime = ei->i_mtime;
%  	ip->i_ctime = ei->i_ctime;
%  	ip->i_flags = 0;
% -	ip->i_flags |= (ei->i_flags & EXT2_APPEND_FL) ? APPEND : 0;
% -	ip->i_flags |= (ei->i_flags & EXT2_IMMUTABLE_FL) ? IMMUTABLE : 0;

I think it would work to just map EXT2*_FL to SF_*.

% +	switch (userflags) {
% +	case 0:
% +		/*
% +		 * Only the superuser may set/clear these flags.
% +		 * This is the current behavior on Linux.
% +		 */
% +		if (ei->i_flags & EXT2_APPEND_FL)
% +			ip->i_flags |= SF_APPEND;
% +		if (ei->i_flags & EXT2_IMMUTABLE_FL)
% +			ip->i_flags |= SF_IMMUTABLE;
% +		break;
% +	case 1:
% +		/*
% +		 * Users may set/clear these flags on files they own.
% +		 */
% +		if (ei->i_flags & EXT2_APPEND_FL)
% +			ip->i_flags |= UF_APPEND;
% +		if (ei->i_flags & EXT2_IMMUTABLE_FL)
% +			ip->i_flags |= UF_IMMUTABLE;
% +		break;

For administration it can be convenient for the file system to behave
a little differently to native mode, but I letting root change the
(system) immutable flags is enough.

% +	case -1:
% +	default:
% +		/*
% +		 * Default behavior on FreeBSD
% +		 */
% +		if (ei->i_flags & EXT2_APPEND_FL)
% +			ip->i_flags |= APPEND;
% +		if (ei->i_flags & EXT2_IMMUTABLE_FL)
% +			ip->i_flags |= IMMUTABLE;
% +		break;
% +	}

I think the current behaviour is too buggy to keep.  (Your original PR
describes the bugs -- FreeBSD makes a mess by setting 2 flags in the
in-core inode and allowing these flags to be changed independently, then
cannot merge the flags properly when writing to the on-disk inode.)

[conversion to the on-disk inode]

Similarly.

There is a problem in ext2_vnops.c that is not touched by these patches.
Even in the simple version that only support SF_*, ext2_setattr() needs
changes to disallow setting of UF_* -- otherwise FreeBSD still makes a
mess, with stat() showing changes to UF_* succeeding while the inode is
in-core but going away when the inode is flushed from the inode/vnode
cache.  The fix is simply to remove the code that supports UF_*.  (We
could also globably replace IMMUTABLE and APPEND by SF_IMMUTABLE and
SF_APPEND.  This would be clearer but would increase divergence from
ffs.)  The fix to support all 3 sysctl modes is not so simple:
- case 0: dynamically disallow attempts to set UF_*.
- case 1: dynamically disallow attempts to set SF_*.
- case -1: dynamically convert attempts to set SF_* or UF*_ into attempts
   to set both, and somehow relax the checks for setting SF_* so that this
   has a chance of succeeding for non-root.
I don't want these complications.

Note that the corresponding code in ffs is poorly organized and buggy
(it doesn't allow preservation of flags in the right way).  ext2_setattr()
was once almost identical to ufs_settatr() but now has the following
bitrot:
- missing support for setting atimes on exec.
- different comments about privilege though the code is the same.
- different comments about truncate() on r/o file systems.  Missing a
   critical fix for truncate().
- missing the comment expansion and cleanup for utimes().  I think there was
   a minor security-related fix in there too, but this is now null.

% --- src/sys/gnu/fs/ext2fs/ext2_vnops.c.orig	2006-02-19 20:53:14.000000000 -0400
% +++ src/sys/gnu/fs/ext2fs/ext2_vnops.c	2008-05-28 07:58:02.189157441 -0430
% @@ -358,6 +358,8 @@ ext2_getattr(ap)
%  	vap->va_mtime.tv_nsec = ip->i_mtimensec;
%  	vap->va_ctime.tv_sec = ip->i_ctime;
%  	vap->va_ctime.tv_nsec = ip->i_ctimensec;
% +	vap->va_birthtime.tv_sec = 0;
% +	vap->va_birthtime.tv_nsec = 0;
%  	vap->va_flags = ip->i_flags;
%  	vap->va_gen = ip->i_gen;
%  	vap->va_blocksize = vp->v_mount->mnt_stat.f_iosize;

This is unrelated and should be handled centrally.  Almost all file
systems get this wrong.  Most fail to set va_birthtime, so stat()
returns kernel stack garbage for st_birthtime.  ffs1 does the same
as the above.  msdosfs does the above correctly, by setting tv_sec to
(time_t)-1 in unsupported cases.

Bruce
Comment 7 Bruce Evans freebsd_committer freebsd_triage 2008-06-02 11:30:02 UTC
On Mon, 2 Jun 2008, Julian Elischer wrote:

> Ighighi wrote:
>> On Linux, only the root user may set/clear the immutable/append flags
>> on ext2 filesystems... Shouldn't FreeBSD do this too, as a POLA?
>
> No I think it should preserver the BSD scheme where being able to
> change the immutable bits is controlled by the system secure level.
> (and your UID of course). At least I think that is what I would
> expect. (All file systems to behave about the same for a
> particular OS.

No, the securelevel already controls things, and the BSD scheme reduces
to only allowing root (strictly, processes with appropriate privilege,
as restricted by securelevel and jails etc, but never mere users), to
change immutable bits, because ext2fs doesn't have any user immutable
bits to change (except phantom bits due to bugs in the current FreeBSD
implementation).

Bruce
Comment 8 mckusick 2008-06-08 20:04:26 UTC
Bruce,

I concur with your analysis of what should be done here. Disallow
manipulation of UF_ flags in ext2 and restrict SF_ flags to appropriate
priviledge as is done in both FreeBSD and Linux. The only debate is
whether to enforce FreeBSD securelevel for ext2 which I would be
inclined to do.

	Kirk McKusick
Comment 9 Mark Linimon freebsd_committer freebsd_triage 2009-05-18 05:31:25 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-fs

Over to maintainer(s).
Comment 10 aditya sarawgi 2009-05-29 11:27:13 UTC
I have created a patch for this. Its a simple patch that maps EXT2_* flags to only SF_* flags and disallows setting of UF_* flags with an EOPNOTSUPP 
error. 

--
Aditya Sarawgi
Comment 11 Jaakko Heinonen freebsd_committer freebsd_triage 2009-11-02 15:17:43 UTC
Responsible Changed
From-To: freebsd-fs->jh

Take.
Comment 12 dfilter service freebsd_committer freebsd_triage 2009-11-05 04:51:50 UTC
Author: jh
Date: Thu Nov  5 04:51:38 2009
New Revision: 198940
URL: http://svn.freebsd.org/changeset/base/198940

Log:
  File flags handling fixes for ext2fs:
  
  - Disallow setting of flags not supported by ext2fs.
  - Map EXT2_APPEND_FL to SF_APPEND.
  - Map EXT2_IMMUTABLE_FL to SF_IMMUTABLE.
  - Map EXT2_NODUMP_FL to UF_NODUMP.
  
  Note that ext2fs doesn't support user settable append and immutable flags.
  EXT2_NODUMP_FL is an user settable flag also on Linux.
  
  PR:		kern/122047
  Reported by:	Ighighi
  Submitted by:	Aditya Sarawgi (original version)
  Reviewed by:	bde
  Approved by:	trasz (mentor)

Modified:
  head/sys/gnu/fs/ext2fs/ext2_inode_cnv.c
  head/sys/gnu/fs/ext2fs/ext2_vnops.c

Modified: head/sys/gnu/fs/ext2fs/ext2_inode_cnv.c
==============================================================================
--- head/sys/gnu/fs/ext2fs/ext2_inode_cnv.c	Thu Nov  5 03:54:03 2009	(r198939)
+++ head/sys/gnu/fs/ext2fs/ext2_inode_cnv.c	Thu Nov  5 04:51:38 2009	(r198940)
@@ -83,8 +83,9 @@ ext2_ei2i(ei, ip)
 	ip->i_mtime = ei->i_mtime;
 	ip->i_ctime = ei->i_ctime;
 	ip->i_flags = 0;
-	ip->i_flags |= (ei->i_flags & EXT2_APPEND_FL) ? APPEND : 0;
-	ip->i_flags |= (ei->i_flags & EXT2_IMMUTABLE_FL) ? IMMUTABLE : 0;
+	ip->i_flags |= (ei->i_flags & EXT2_APPEND_FL) ? SF_APPEND : 0;
+	ip->i_flags |= (ei->i_flags & EXT2_IMMUTABLE_FL) ? SF_IMMUTABLE : 0;
+	ip->i_flags |= (ei->i_flags & EXT2_NODUMP_FL) ? UF_NODUMP : 0;
 	ip->i_blocks = ei->i_blocks;
 	ip->i_gen = ei->i_generation;
 	ip->i_uid = ei->i_uid;
@@ -121,8 +122,9 @@ ext2_i2ei(ip, ei)
 	ei->i_ctime = ip->i_ctime;
 	ei->i_flags = ip->i_flags;
 	ei->i_flags = 0;
-	ei->i_flags |= (ip->i_flags & APPEND) ? EXT2_APPEND_FL: 0;
-	ei->i_flags |= (ip->i_flags & IMMUTABLE) ? EXT2_IMMUTABLE_FL: 0;
+	ei->i_flags |= (ip->i_flags & SF_APPEND) ? EXT2_APPEND_FL: 0;
+	ei->i_flags |= (ip->i_flags & SF_IMMUTABLE) ? EXT2_IMMUTABLE_FL: 0;
+	ei->i_flags |= (ip->i_flags & UF_NODUMP) ? EXT2_NODUMP_FL : 0;
 	ei->i_blocks = ip->i_blocks;
 	ei->i_generation = ip->i_gen;
 	ei->i_uid = ip->i_uid;

Modified: head/sys/gnu/fs/ext2fs/ext2_vnops.c
==============================================================================
--- head/sys/gnu/fs/ext2fs/ext2_vnops.c	Thu Nov  5 03:54:03 2009	(r198939)
+++ head/sys/gnu/fs/ext2fs/ext2_vnops.c	Thu Nov  5 04:51:38 2009	(r198940)
@@ -391,6 +391,10 @@ ext2_setattr(ap)
 		return (EINVAL);
 	}
 	if (vap->va_flags != VNOVAL) {
+		/* Disallow flags not supported by ext2fs. */
+		if (vap->va_flags & ~(SF_APPEND | SF_IMMUTABLE | UF_NODUMP))
+			return (EOPNOTSUPP);
+
 		if (vp->v_mount->mnt_flag & MNT_RDONLY)
 			return (EROFS);
 		/*
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 13 Jaakko Heinonen freebsd_committer freebsd_triage 2009-11-05 05:10:05 UTC
State Changed
From-To: open->patched

Patched in head (r198940).
Comment 14 dfilter service freebsd_committer freebsd_triage 2010-01-04 14:35:46 UTC
Author: jh
Date: Mon Jan  4 14:35:36 2010
New Revision: 201497
URL: http://svn.freebsd.org/changeset/base/201497

Log:
  MFC r198940:
  
  File flags handling fixes for ext2fs:
  
  - Disallow setting of flags not supported by ext2fs.
  - Map EXT2_APPEND_FL to SF_APPEND.
  - Map EXT2_IMMUTABLE_FL to SF_IMMUTABLE.
  - Map EXT2_NODUMP_FL to UF_NODUMP.
  
  Note that ext2fs doesn't support user settable append and immutable flags.
  EXT2_NODUMP_FL is an user settable flag also on Linux.
  
  PR:		kern/122047
  Approved by:	trasz (mentor)

Modified:
  stable/8/sys/gnu/fs/ext2fs/ext2_inode_cnv.c
  stable/8/sys/gnu/fs/ext2fs/ext2_vnops.c
Directory Properties:
  stable/8/sys/   (props changed)
  stable/8/sys/amd64/include/xen/   (props changed)
  stable/8/sys/cddl/contrib/opensolaris/   (props changed)
  stable/8/sys/contrib/dev/acpica/   (props changed)
  stable/8/sys/contrib/pf/   (props changed)
  stable/8/sys/dev/xen/xenpci/   (props changed)

Modified: stable/8/sys/gnu/fs/ext2fs/ext2_inode_cnv.c
==============================================================================
--- stable/8/sys/gnu/fs/ext2fs/ext2_inode_cnv.c	Mon Jan  4 14:34:01 2010	(r201496)
+++ stable/8/sys/gnu/fs/ext2fs/ext2_inode_cnv.c	Mon Jan  4 14:35:36 2010	(r201497)
@@ -83,8 +83,9 @@ ext2_ei2i(ei, ip)
 	ip->i_mtime = ei->i_mtime;
 	ip->i_ctime = ei->i_ctime;
 	ip->i_flags = 0;
-	ip->i_flags |= (ei->i_flags & EXT2_APPEND_FL) ? APPEND : 0;
-	ip->i_flags |= (ei->i_flags & EXT2_IMMUTABLE_FL) ? IMMUTABLE : 0;
+	ip->i_flags |= (ei->i_flags & EXT2_APPEND_FL) ? SF_APPEND : 0;
+	ip->i_flags |= (ei->i_flags & EXT2_IMMUTABLE_FL) ? SF_IMMUTABLE : 0;
+	ip->i_flags |= (ei->i_flags & EXT2_NODUMP_FL) ? UF_NODUMP : 0;
 	ip->i_blocks = ei->i_blocks;
 	ip->i_gen = ei->i_generation;
 	ip->i_uid = ei->i_uid;
@@ -121,8 +122,9 @@ ext2_i2ei(ip, ei)
 	ei->i_ctime = ip->i_ctime;
 	ei->i_flags = ip->i_flags;
 	ei->i_flags = 0;
-	ei->i_flags |= (ip->i_flags & APPEND) ? EXT2_APPEND_FL: 0;
-	ei->i_flags |= (ip->i_flags & IMMUTABLE) ? EXT2_IMMUTABLE_FL: 0;
+	ei->i_flags |= (ip->i_flags & SF_APPEND) ? EXT2_APPEND_FL: 0;
+	ei->i_flags |= (ip->i_flags & SF_IMMUTABLE) ? EXT2_IMMUTABLE_FL: 0;
+	ei->i_flags |= (ip->i_flags & UF_NODUMP) ? EXT2_NODUMP_FL : 0;
 	ei->i_blocks = ip->i_blocks;
 	ei->i_generation = ip->i_gen;
 	ei->i_uid = ip->i_uid;

Modified: stable/8/sys/gnu/fs/ext2fs/ext2_vnops.c
==============================================================================
--- stable/8/sys/gnu/fs/ext2fs/ext2_vnops.c	Mon Jan  4 14:34:01 2010	(r201496)
+++ stable/8/sys/gnu/fs/ext2fs/ext2_vnops.c	Mon Jan  4 14:35:36 2010	(r201497)
@@ -391,6 +391,10 @@ ext2_setattr(ap)
 		return (EINVAL);
 	}
 	if (vap->va_flags != VNOVAL) {
+		/* Disallow flags not supported by ext2fs. */
+		if (vap->va_flags & ~(SF_APPEND | SF_IMMUTABLE | UF_NODUMP))
+			return (EOPNOTSUPP);
+
 		if (vp->v_mount->mnt_flag & MNT_RDONLY)
 			return (EROFS);
 		/*
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 15 dfilter service freebsd_committer freebsd_triage 2010-04-07 16:33:38 UTC
Author: jh
Date: Wed Apr  7 15:33:19 2010
New Revision: 206359
URL: http://svn.freebsd.org/changeset/base/206359

Log:
  MFC r198940:
  
  File flags handling fixes for ext2fs:
  
  - Disallow setting of flags not supported by ext2fs.
  - Map EXT2_APPEND_FL to SF_APPEND.
  - Map EXT2_IMMUTABLE_FL to SF_IMMUTABLE.
  - Map EXT2_NODUMP_FL to UF_NODUMP.
  
  Note that ext2fs doesn't support user settable append and immutable flags.
  EXT2_NODUMP_FL is an user settable flag also on Linux.
  
  PR:		kern/122047

Modified:
  stable/7/sys/gnu/fs/ext2fs/ext2_inode_cnv.c
  stable/7/sys/gnu/fs/ext2fs/ext2_vnops.c
Directory Properties:
  stable/7/sys/   (props changed)
  stable/7/sys/cddl/contrib/opensolaris/   (props changed)
  stable/7/sys/contrib/dev/acpica/   (props changed)
  stable/7/sys/contrib/pf/   (props changed)

Modified: stable/7/sys/gnu/fs/ext2fs/ext2_inode_cnv.c
==============================================================================
--- stable/7/sys/gnu/fs/ext2fs/ext2_inode_cnv.c	Wed Apr  7 15:29:13 2010	(r206358)
+++ stable/7/sys/gnu/fs/ext2fs/ext2_inode_cnv.c	Wed Apr  7 15:33:19 2010	(r206359)
@@ -83,8 +83,9 @@ ext2_ei2i(ei, ip)
 	ip->i_mtime = ei->i_mtime;
 	ip->i_ctime = ei->i_ctime;
 	ip->i_flags = 0;
-	ip->i_flags |= (ei->i_flags & EXT2_APPEND_FL) ? APPEND : 0;
-	ip->i_flags |= (ei->i_flags & EXT2_IMMUTABLE_FL) ? IMMUTABLE : 0;
+	ip->i_flags |= (ei->i_flags & EXT2_APPEND_FL) ? SF_APPEND : 0;
+	ip->i_flags |= (ei->i_flags & EXT2_IMMUTABLE_FL) ? SF_IMMUTABLE : 0;
+	ip->i_flags |= (ei->i_flags & EXT2_NODUMP_FL) ? UF_NODUMP : 0;
 	ip->i_blocks = ei->i_blocks;
 	ip->i_gen = ei->i_generation;
 	ip->i_uid = ei->i_uid;
@@ -121,8 +122,9 @@ ext2_i2ei(ip, ei)
 	ei->i_ctime = ip->i_ctime;
 	ei->i_flags = ip->i_flags;
 	ei->i_flags = 0;
-	ei->i_flags |= (ip->i_flags & APPEND) ? EXT2_APPEND_FL: 0;
-	ei->i_flags |= (ip->i_flags & IMMUTABLE) ? EXT2_IMMUTABLE_FL: 0;
+	ei->i_flags |= (ip->i_flags & SF_APPEND) ? EXT2_APPEND_FL: 0;
+	ei->i_flags |= (ip->i_flags & SF_IMMUTABLE) ? EXT2_IMMUTABLE_FL: 0;
+	ei->i_flags |= (ip->i_flags & UF_NODUMP) ? EXT2_NODUMP_FL : 0;
 	ei->i_blocks = ip->i_blocks;
 	ei->i_generation = ip->i_gen;
 	ei->i_uid = ip->i_uid;

Modified: stable/7/sys/gnu/fs/ext2fs/ext2_vnops.c
==============================================================================
--- stable/7/sys/gnu/fs/ext2fs/ext2_vnops.c	Wed Apr  7 15:29:13 2010	(r206358)
+++ stable/7/sys/gnu/fs/ext2fs/ext2_vnops.c	Wed Apr  7 15:33:19 2010	(r206359)
@@ -399,6 +399,10 @@ ext2_setattr(ap)
 		return (EINVAL);
 	}
 	if (vap->va_flags != VNOVAL) {
+		/* Disallow flags not supported by ext2fs. */
+		if (vap->va_flags & ~(SF_APPEND | SF_IMMUTABLE | UF_NODUMP))
+			return (EOPNOTSUPP);
+
 		if (vp->v_mount->mnt_flag & MNT_RDONLY)
 			return (EROFS);
 		/*
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 16 Jaakko Heinonen freebsd_committer freebsd_triage 2010-04-07 16:52:14 UTC
State Changed
From-To: patched->closed

Fixed in head, stable/8 and stable/7.