2006-02-08 01:12:36

by Bernd Schubert

[permalink] [raw]
Subject: 2.6.15 Bug? New security model?

Hi,

just wanted to upgrade from 2.6.13 to 2.6.15, but there is a really BIG
problem. I just can't write into *some* directories and don't see the reason.

With 2.6.15:
bathl:~# touch /var/run/test
touch: cannot touch `/var/run/test': Permission denied

With 2.6.13:
bathl:~# touch /var/run/test
(No error message)

The strace and ltrace outputs to the touch command are attached.
The /proc/mounts and kernel configuration as well. Sorry, no full dmesg
output - there was really nothing interesting at the end and to get the the
beginning I have to increase the message size.

Already run a reiserfsck from a boot cdrom, it didn't find anything. Booted
serveral times 2.6.13 and 2.6.15, always only 2.6.15 has the problem.

The first directory I noticed to have a problem was /dev. Though everything
inside were proper character and block devices, the system even couldn't
write to /dev/null. Also moving this directory was refused.
After the problem first came up, I first booted knoppix, checked the
filesystem, created a new /dev and copied everything from the old /dev to the
new one. Rebooted and now /dev was fixed. However, since the system now can't
write to /var/run, the (debian) initscripts are rather confused and can't
start many processes. There are many other directories affected as well and I
wouldn't like to use the copy method.


Any ideas?

Thanks,
Bernd


--
Bernd Schubert
PCI / Theoretische Chemie
Universit?t Heidelberg
INF 229
69120 Heidelberg


Attachments:
(No filename) (1.45 kB)
strace.out (3.78 kB)
ltrace.out (1.30 kB)
mounts (501.00 B)
config (38.63 kB)
Download all attachments

2006-02-08 01:31:59

by Sam Vilain

[permalink] [raw]
Subject: Re: 2.6.15 Bug? New security model?

Bernd Schubert wrote:
> Hi,
>
> just wanted to upgrade from 2.6.13 to 2.6.15, but there is a really BIG
> problem. I just can't write into *some* directories and don't see the reason.
>
> With 2.6.15:
> bathl:~# touch /var/run/test
> touch: cannot touch `/var/run/test': Permission denied
>
> With 2.6.13:
> bathl:~# touch /var/run/test
> (No error message)

Some ideas; ACLs, SELinux, Attributes, Capabilities.

Sam.

2006-02-08 05:37:36

by John M Flinchbaugh

[permalink] [raw]
Subject: Re: 2.6.15 Bug? New security model?

On Wed, Feb 08, 2006 at 02:31:46PM +1300, Sam Vilain wrote:
> Bernd Schubert wrote:
> >With 2.6.15:
> >bathl:~# touch /var/run/test
> >touch: cannot touch `/var/run/test': Permission denied
> >With 2.6.13:
> >bathl:~# touch /var/run/test
> >(No error message)
>
> Some ideas; ACLs, SELinux, Attributes, Capabilities.

lsattr -d /var/run && lsattr /var/run

I saw very similar things going from 2.6.15.1 to 2.6.15.2. 2.6.15.2's
changelog advertises a fix to reenable extended attributes on reiserfs.
On one machine this is fine, and lsattr shows no attributes enabled
(----------), but on another machine, I ended up with all sorts of crazy
attributes set seemingly randomly -- compression, experimental flags,
immutable, append-only, all over the map.

I tried clearing them (chattr -R = /var ...etc), but I still found a
file here and there which refused to be removed, even though lsattr
showed no flags for it. After a restart or 2, I saw some attributes
revert back and I started having trouble removing files from /var/run
and other places again.

I ended up reverting back to 2.6.15.1 until I have a chance to
investigate further and try to come up with something reportable. In
2.6.15.1, attributes didn't work at all, giving an ioctl error, though
the same kernel options were used. I suspect this is the fix to which
the Changelog is referring.

I must wonder if I'm suffering from some sort of fs corruption which
only manifests itself in the attribute settings, and which a reisefsck
doesn't recognize or correct. I could be tempted to recreate the
filesystems from scratch to see if they still have issues.
--
John M Flinchbaugh
[email protected]


Attachments:
(No filename) (1.62 kB)
signature.asc (189.00 B)
Digital signature
Download all attachments

2006-02-08 12:15:05

by Bernd Schubert

[permalink] [raw]
Subject: Re: 2.6.15 Bug? New security model?

CC'ed also the reiser-list


On Wednesday 08 February 2006 06:37, John M Flinchbaugh wrote:
> On Wed, Feb 08, 2006 at 02:31:46PM +1300, Sam Vilain wrote:
> > Bernd Schubert wrote:
> > >With 2.6.15:
> > >bathl:~# touch /var/run/test
> > >touch: cannot touch `/var/run/test': Permission denied
> > >With 2.6.13:
> > >bathl:~# touch /var/run/test
> > >(No error message)
> >
> > Some ideas; ACLs, SELinux, Attributes, Capabilities.
>
> lsattr -d /var/run && lsattr /var/run

Indeed, with 2.6.13

bathl:~# lsattr -d /var/run
lsattr: Inappropriate ioctl for device While reading flags on /var/run

with 2.6.15.3

bathl:~# cat lsatr.out.2.6.15
--S-ia-AcBZXEj-t- /var/run


After the problem came up, I already suspected something like this and
therefore already had the kernel recompiled without xattr support, so I
don't know why lsattr shows something for 2.6.15 and nothing for 2.6.13.

here the reiser part from the kernel config of 2.6.13

CONFIG_REISERFS_FS=y
# CONFIG_REISERFS_CHECK is not set
CONFIG_REISERFS_PROC_INFO=y
CONFIG_REISERFS_FS_XATTR=y
CONFIG_REISERFS_FS_POSIX_ACL=y
CONFIG_REISERFS_FS_SECURITY=y
# CONFIG_JFS_FS is not set
CONFIG_FS_POSIX_ACL=y

here 2.6.15

CONFIG_REISERFS_FS=y
# CONFIG_REISERFS_CHECK is not set
CONFIG_REISERFS_PROC_INFO=y
# CONFIG_REISERFS_FS_XATTR is not set
# CONFIG_JFS_FS is not set
CONFIG_FS_POSIX_ACL=y
# CONFIG_XFS_FS is not set
# CONFIG_MINIX_FS is not set
# CONFIG_ROMFS_FS is not set


Anyway, I never set any attributes using chattr, so these seem to be random
attributes, nice.



>
> I saw very similar things going from 2.6.15.1 to 2.6.15.2. 2.6.15.2's
> changelog advertises a fix to reenable extended attributes on reiserfs.
> On one machine this is fine, and lsattr shows no attributes enabled
> (----------), but on another machine, I ended up with all sorts of crazy
> attributes set seemingly randomly -- compression, experimental flags,
> immutable, append-only, all over the map.
>
> I tried clearing them (chattr -R = /var ...etc), but I still found a
> file here and there which refused to be removed, even though lsattr
> showed no flags for it. After a restart or 2, I saw some attributes
> revert back and I started having trouble removing files from /var/run
> and other places again.
>
> I ended up reverting back to 2.6.15.1 until I have a chance to
> investigate further and try to come up with something reportable. In
> 2.6.15.1, attributes didn't work at all, giving an ioctl error, though
> the same kernel options were used. I suspect this is the fix to which
> the Changelog is referring.
>
> I must wonder if I'm suffering from some sort of fs corruption which
> only manifests itself in the attribute settings, and which a reisefsck
> doesn't recognize or correct. I could be tempted to recreate the
> filesystems from scratch to see if they still have issues.

--
Bernd Schubert
PCI / Theoretische Chemie
Universit?t Heidelberg
INF 229
69120 Heidelberg

2006-02-08 20:50:56

by Chris Wright

[permalink] [raw]
Subject: Re: 2.6.15 Bug? New security model?

* Bernd Schubert ([email protected]) wrote:
> On Wednesday 08 February 2006 06:37, John M Flinchbaugh wrote:
> > On Wed, Feb 08, 2006 at 02:31:46PM +1300, Sam Vilain wrote:
> > > Bernd Schubert wrote:
> > > >With 2.6.15:
> > > >bathl:~# touch /var/run/test
> > > >touch: cannot touch `/var/run/test': Permission denied
> > > >With 2.6.13:
> > > >bathl:~# touch /var/run/test
> > > >(No error message)
> > >
> > > Some ideas; ACLs, SELinux, Attributes, Capabilities.
> >
> > lsattr -d /var/run && lsattr /var/run
>
> Indeed, with 2.6.13
>
> bathl:~# lsattr -d /var/run
> lsattr: Inappropriate ioctl for device While reading flags on /var/run
>
> with 2.6.15.3

OK, this has a reiserfs fix for attrs support. Rather than back it
out, I'd like to get the proper fix.

> bathl:~# cat lsatr.out.2.6.15
> --S-ia-AcBZXEj-t- /var/run
>
> After the problem came up, I already suspected something like this and
> therefore already had the kernel recompiled without xattr support, so I
> don't know why lsattr shows something for 2.6.15 and nothing for 2.6.13.

attrs != xattrs

Couple of things:

1) what does 'grep attrs_cleared /proc/fs/reiserfs/on-disk-super' show?

2) does mount -o attrs ... make a difference?

thanks,
-chris

2006-02-08 21:46:20

by Bernd Schubert

[permalink] [raw]
Subject: Re: 2.6.15 Bug? New security model?

> >
> > After the problem came up, I already suspected something like this and
> > therefore already had the kernel recompiled without xattr support, so I
> > don't know why lsattr shows something for 2.6.15 and nothing for 2.6.13.
>
> attrs != xattrs

Ah, I thought its the same.

>
> Couple of things:
>
> 1) what does 'grep attrs_cleared /proc/fs/reiserfs/on-disk-super' show?

Er, you mean /proc/fs/reiserfs/{partition}/on-disk-super?

bernd@bathl ~>grep attrs_cleared /proc/fs/reiserfs/hda6/on-disk-super
flags: 1[attrs_cleared]


>
> 2) does mount -o attrs ... make a difference?

Yes, 2.6.13 now makes the same trouble. No difference with 2.6.15.3.
I played with mount -o noattrs, this makes no difference with 2.6.13, but has
some effects to 2.6.15.3. Creating files in /var/run is possible again,
lsattr gives "lsattr: Inappropriate ioctl for device While reading flags
on /var/run", but deleting files in /var/run is still impossible (still
rather bad for the init-scripts).


Thanks,
Bernd

--
Bernd Schubert
PCI / Theoretische Chemie
Universit?t Heidelberg
INF 229
69120 Heidelberg

2006-02-08 22:04:36

by Chris Wright

[permalink] [raw]
Subject: Re: 2.6.15 Bug? New security model?

* Bernd Schubert ([email protected]) wrote:
> Er, you mean /proc/fs/reiserfs/{partition}/on-disk-super?

Yup.

> bernd@bathl ~>grep attrs_cleared /proc/fs/reiserfs/hda6/on-disk-super
> flags: 1[attrs_cleared]

> > 2) does mount -o attrs ... make a difference?
>
> Yes, 2.6.13 now makes the same trouble. No difference with 2.6.15.3.
> I played with mount -o noattrs, this makes no difference with 2.6.13, but has
> some effects to 2.6.15.3. Creating files in /var/run is possible again,
> lsattr gives "lsattr: Inappropriate ioctl for device While reading flags
> on /var/run", but deleting files in /var/run is still impossible (still
> rather bad for the init-scripts).

Yes, that's what I thought. There's still some backward logic in there.
noattrs vs. attrs triggers whether the code path that's patched in
2.6.15.3 is taken. I'll dig a bit more, but hopefully the reiserfs folks
can fix this for us.

thanks,
-chris

2006-02-11 21:56:14

by Sergey Vlasov

[permalink] [raw]
Subject: Re: 2.6.15 Bug? New security model?

On Wed, 8 Feb 2006 14:11:24 -0800 Chris Wright wrote:

> * Bernd Schubert ([email protected]) wrote:
> > Er, you mean /proc/fs/reiserfs/{partition}/on-disk-super?
>
> Yup.
>
> > bernd@bathl ~>grep attrs_cleared /proc/fs/reiserfs/hda6/on-disk-super
> > flags: 1[attrs_cleared]
>
> > > 2) does mount -o attrs ... make a difference?
> >
> > Yes, 2.6.13 now makes the same trouble. No difference with 2.6.15.3.
> > I played with mount -o noattrs, this makes no difference with 2.6.13, but has
> > some effects to 2.6.15.3. Creating files in /var/run is possible again,
> > lsattr gives "lsattr: Inappropriate ioctl for device While reading flags
> > on /var/run", but deleting files in /var/run is still impossible (still
> > rather bad for the init-scripts).

Is the filesystem in question old - could it be created initially in the
reiserfs v3.5 format (used with 2.2.x kernels) and later converted to
v3.6 (by mounting with the "conv" option)?

> Yes, that's what I thought. There's still some backward logic in there.
> noattrs vs. attrs triggers whether the code path that's patched in
> 2.6.15.3 is taken. I'll dig a bit more, but hopefully the reiserfs folks
> can fix this for us.

Here is a simple test case which reproduces the problem with a
filesystem converted from v3.5:

# dd if=/dev/zero of=tmp.img bs=1M count=100
# mkreiserfs --format 3.5 -f tmp.img
# mount -t reiserfs -o loop,conv tmp.img /mnt/disk/
# umount /mnt/disk/
# reiserfsck --clean-attributes tmp.img
# mount -t reiserfs -o loop tmp.img /mnt/disk/

At this point, I get obviously wrong attributes on /mnt/disk:

# lsattr -d /mnt/disk/
-----a--c---- /mnt/disk/

BTW, this breaks even with kernels earlier than 2.6.15.2, if you also
add the "attrs" options to the last mount command.

Apparently the reiserfs attrs code has been broken for such converted
filesystems for some time, but it could be enabled only with the "attrs"
option, so people were not hitting this. However, the following patch:

http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=2949ccf9379678df66ecf2ca70ed4656159eacdd

changed the logic to enable the "attrs" option on all filesystems which
have the reiserfs_attrs_cleared flag. But that patch was broken - it
did not really set the option properly, so the attrs-related breakage
did not became visible until yet another patch:

http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=d35c602870ece3166cff3d25fbc687a7f707acf3

which later made into 2.6.15.2, and caused problems for some people.

I have noticed that fs/reiserfs/inode.c:init_inode() does not initialize
REISERFS_I(inode)->i_attrs and inode->i_flags (as done by
sd_attrs_to_i_attrs()) in the branch for v1 stat data; maybe this causes
the problem?


Attachments:
(No filename) (2.72 kB)
(No filename) (190.00 B)
Download all attachments

2006-02-11 23:50:38

by Bernd Schubert

[permalink] [raw]
Subject: Re: 2.6.15 Bug? New security model?

Privet!

> > > Yes, 2.6.13 now makes the same trouble. No difference with 2.6.15.3.
> > > I played with mount -o noattrs, this makes no difference with 2.6.13,
> > > but has some effects to 2.6.15.3. Creating files in /var/run is
> > > possible again, lsattr gives "lsattr: Inappropriate ioctl for device
> > > While reading flags on /var/run", but deleting files in /var/run is
> > > still impossible (still rather bad for the init-scripts).
>
> Is the filesystem in question old - could it be created initially in the
> reiserfs v3.5 format (used with 2.2.x kernels) and later converted to
> v3.6 (by mounting with the "conv" option)?

Well, I'm rather sure I created this filesystem in May 2001 after the Win2000
partion manager selectively deleted all my ext2 partitions (I'm still very
angry about MS). However, that time I already knew that v3.5 should not be
used with NFS and I always used NFS a lot(my first mail to the reiser list
was in early 2001 about a reiser v3.5 + nfs problem). I also used
Slackware-8.0-pre that time and also just checked it, the mkreiserfs from
slackware-8.0 has v3.6 as default. So really, its very unlikely that my
current root partition ever had v3.5 on it.

>
> > Yes, that's what I thought. There's still some backward logic in there.
> > noattrs vs. attrs triggers whether the code path that's patched in
> > 2.6.15.3 is taken. I'll dig a bit more, but hopefully the reiserfs folks
> > can fix this for us.
>
> Here is a simple test case which reproduces the problem with a
> filesystem converted from v3.5:
>
> # dd if=/dev/zero of=tmp.img bs=1M count=100
> # mkreiserfs --format 3.5 -f tmp.img
> # mount -t reiserfs -o loop,conv tmp.img /mnt/disk/
> # umount /mnt/disk/
> # reiserfsck --clean-attributes tmp.img
> # mount -t reiserfs -o loop tmp.img /mnt/disk/
>
> At this point, I get obviously wrong attributes on /mnt/disk:
>
> # lsattr -d /mnt/disk/
> -----a--c---- /mnt/disk/
>
> BTW, this breaks even with kernels earlier than 2.6.15.2, if you also
> add the "attrs" options to the last mount command.
>
> Apparently the reiserfs attrs code has been broken for such converted
> filesystems for some time, but it could be enabled only with the "attrs"
> option, so people were not hitting this. However, the following patch:
>
> http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=comm
>it;h=2949ccf9379678df66ecf2ca70ed4656159eacdd
>
> changed the logic to enable the "attrs" option on all filesystems which
> have the reiserfs_attrs_cleared flag. But that patch was broken - it
> did not really set the option properly, so the attrs-related breakage
> did not became visible until yet another patch:
>
> http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=comm
>it;h=d35c602870ece3166cff3d25fbc687a7f707acf3
>
> which later made into 2.6.15.2, and caused problems for some people.

I already reverted the patch in 2.6.15.2 for some NFSv4 tests with 2.6.15, but
of course, this is not the final solution. Neither for me, nor for the other
thousands of reiserfs filesystems out there, that suddenly break beginning
with 2.6.15.2.


>
> I have noticed that fs/reiserfs/inode.c:init_inode() does not initialize
> REISERFS_I(inode)->i_attrs and inode->i_flags (as done by
> sd_attrs_to_i_attrs()) in the branch for v1 stat data; maybe this causes
> the problem?

Please forgive me my missing knowledge about the internals of reiserfs,
reiserfs doesn't have an inode for each file and directory as ext2, right?
Is there some way to detect if the inode was created on reiser3.5?

Thanks,
Bernd

--
Bernd Schubert
PCI / Theoretische Chemie
Universit?t Heidelberg
INF 229
69120 Heidelberg

2006-02-12 17:57:44

by Jeff Mahoney

[permalink] [raw]
Subject: Re: 2.6.15 Bug? New security model?

On Sun, Feb 12, 2006 at 12:55:41AM +0300, Sergey Vlasov wrote:
> I have noticed that fs/reiserfs/inode.c:init_inode() does not initialize
> REISERFS_I(inode)->i_attrs and inode->i_flags (as done by
> sd_attrs_to_i_attrs()) in the branch for v1 stat data; maybe this causes
> the problem?

Yes. This would absolutely cause a problem. Thanks for the triage.

The failure to set i_attrs = 0 for the sd v1 path means that *any* new objects
that inherit from a v3.5-created object (-o conv means new objects will be
sd v2, old ones aren't 'updated'), will end up with bogus attributes.

This is essentially code-introduced corruption. The patch to fix it in future
versions is easy enough, but you'll need to run reiserfsck --clean-attributes
<device> on any affected file systems.

Bernd - If you haven't already run reiserfsck --clean-attributes on the fs,
can you please test the attached patch? It adds a check to make sure the
file system has always been v3.6 before enabling the attributes by default.

-Jeff

--
Jeff Mahoney
SuSE Labs


Attachments:
(No filename) (0.00 B)
(No filename) (189.00 B)
Download all attachments

2006-02-12 18:44:36

by Sergey Vlasov

[permalink] [raw]
Subject: Re: 2.6.15 Bug? New security model?

On Sun, Feb 12, 2006 at 12:50:30AM +0100, Bernd Schubert wrote:
> Privet!
>
> > > > Yes, 2.6.13 now makes the same trouble. No difference with 2.6.15.3.
> > > > I played with mount -o noattrs, this makes no difference with 2.6.13,
> > > > but has some effects to 2.6.15.3. Creating files in /var/run is
> > > > possible again, lsattr gives "lsattr: Inappropriate ioctl for device
> > > > While reading flags on /var/run", but deleting files in /var/run is
> > > > still impossible (still rather bad for the init-scripts).
> >
> > Is the filesystem in question old - could it be created initially in the
> > reiserfs v3.5 format (used with 2.2.x kernels) and later converted to
> > v3.6 (by mounting with the "conv" option)?
>
> Well, I'm rather sure I created this filesystem in May 2001 after the Win2000
> partion manager selectively deleted all my ext2 partitions (I'm still very
> angry about MS). However, that time I already knew that v3.5 should not be
> used with NFS and I always used NFS a lot(my first mail to the reiser list
> was in early 2001 about a reiser v3.5 + nfs problem). I also used
> Slackware-8.0-pre that time and also just checked it, the mkreiserfs from
> slackware-8.0 has v3.6 as default. So really, its very unlikely that my
> current root partition ever had v3.5 on it.

Still there is a chance (who knows what has been in 8.0-pre at that
time... and changelog says that reiserfsprogs was updated on May 14
2001).

> > > Yes, that's what I thought. There's still some backward logic in there.
> > > noattrs vs. attrs triggers whether the code path that's patched in
> > > 2.6.15.3 is taken. I'll dig a bit more, but hopefully the reiserfs folks
> > > can fix this for us.
> >
> > Here is a simple test case which reproduces the problem with a
> > filesystem converted from v3.5:
> >
> > # dd if=/dev/zero of=tmp.img bs=1M count=100
> > # mkreiserfs --format 3.5 -f tmp.img

Add here:

# mount -t reiserfs -o loop tmp.img /mnt/disk
# tar -xz -f something.tar.gz -C /mnt/disk
# umount /mnt/disk

Otherwise the test seems to be unreliable.

> > # mount -t reiserfs -o loop,conv tmp.img /mnt/disk/
> > # umount /mnt/disk/
> > # reiserfsck --clean-attributes tmp.img
> > # mount -t reiserfs -o loop tmp.img /mnt/disk/
> >
> > At this point, I get obviously wrong attributes on /mnt/disk:
> >
> > # lsattr -d /mnt/disk/
> > -----a--c---- /mnt/disk/

Sometimes the root directory seems to have correct attrs, but some
files should still get garbage.

> I already reverted the patch in 2.6.15.2 for some NFSv4 tests with 2.6.15, but
> of course, this is not the final solution. Neither for me, nor for the other
> thousands of reiserfs filesystems out there, that suddenly break beginning
> with 2.6.15.2.

Of course - therefore Jeff Mahoney already prepared the patch to
remove the problematic feature (automatic enabling of the "attrs"
mount option) completely:

http://lkml.org/lkml/2006/2/12/76

(Backing out only the latest patch is wrong, because then the "tails"
option will remain broken.)

However, the real bug is somewhere else.

> > I have noticed that fs/reiserfs/inode.c:init_inode() does not initialize
> > REISERFS_I(inode)->i_attrs and inode->i_flags (as done by
> > sd_attrs_to_i_attrs()) in the branch for v1 stat data; maybe this causes
> > the problem?
>
> Please forgive me my missing knowledge about the internals of reiserfs,
> reiserfs doesn't have an inode for each file and directory as ext2, right?
> Is there some way to detect if the inode was created on reiser3.5?

You may try to do something like "chgrp 70000" on it - the v1 stat
data format has 16-bit uid/gid fields and therefore does not support
uid/gid greater than 65535. If "chgrp 70000" fails with "Invalid
argument", but works with a value lower than 65535, most likely the
file in question still has old stat data format. (If both operations
fail, this may be due to broken attrs on the file which prohibit the
operation).


Attachments:
(No filename) (3.86 kB)
(No filename) (189.00 B)
Download all attachments

2006-02-12 19:21:28

by Sergey Vlasov

[permalink] [raw]
Subject: Re: 2.6.15 Bug? New security model?

On Sun, Feb 12, 2006 at 12:57:40PM -0500, Jeff Mahoney wrote:
> diff -ruNpX dontdiff linux-2.6.15/fs/reiserfs/inode.c linux-2.6.15-reiserfs/fs/reiserfs/inode.c
> --- linux-2.6.15/fs/reiserfs/inode.c 2006-02-06 19:54:10.000000000 -0500
> +++ linux-2.6.15-reiserfs/fs/reiserfs/inode.c 2006-02-12 12:43:00.000000000 -0500
> @@ -1195,6 +1195,7 @@ static void init_inode(struct inode *ino
> /* nopack is initially zero for v1 objects. For v2 objects,
> nopack is initialised from sd_attrs */
> REISERFS_I(inode)->i_flags &= ~i_nopack_mask;
> + REISERFS_I(inode)->i_attrs = 0;

This part of the patch works fine for my test case - no more bogus
attributes, even when mounting with the "attrs" option.

> } else {
> // new stat data found, but object may have old items
> // (directories and symlinks)
> diff -ruNpX dontdiff linux-2.6.15/fs/reiserfs/super.c linux-2.6.15-reiserfs/fs/reiserfs/super.c
> --- linux-2.6.15/fs/reiserfs/super.c 2006-02-06 19:54:27.000000000 -0500
> +++ linux-2.6.15-reiserfs/fs/reiserfs/super.c 2006-02-12 12:48:41.000000000 -0500
> @@ -1121,7 +1121,9 @@ static void handle_attrs(struct super_bl
> "reiserfs: cannot support attributes until flag is set in super-block");
> REISERFS_SB(s)->s_mount_opt &= ~(1 << REISERFS_ATTRS);
> }
> - } else if (le32_to_cpu(rs->s_flags) & reiserfs_attrs_cleared) {
> + } else if (le32_to_cpu(rs->s_flags) & reiserfs_attrs_cleared &&
> + get_inode_item_key_version(s->s_root->d_inode) == KEY_FORMAT_3_6) {
> + /* Enable attrs by default on v3.6-native file systems */
> REISERFS_SB(s)->s_mount_opt |= (1 << REISERFS_ATTRS);
> }
> }

This part, however, does not work - apparently the condition is never
true, even on a freshly created 3.6-format filesystem:

# mkreiserfs --format 3.6 -f tmp2.img
# mount -t reiserfs -o loop tmp2.img /mnt/disk/
# lsattr -d /mnt/disk/
lsattr: Inappropriate ioctl for device While reading flags on /mnt/disk/


Apparently directories always have old key format, even on new
filesystems:

if (old_format_only(sb) || S_ISDIR(mode) || S_ISLNK(mode))
set_inode_item_key_version(inode, KEY_FORMAT_3_5);
else
set_inode_item_key_version(inode, KEY_FORMAT_3_6);

However, checking the stat data format works:

diff --git a/fs/reiserfs/inode.c b/fs/reiserfs/inode.c
index b33d67b..a2ea7ed 100644
--- a/fs/reiserfs/inode.c
+++ b/fs/reiserfs/inode.c
@@ -1195,6 +1195,7 @@ static void init_inode(struct inode *ino
/* nopack is initially zero for v1 objects. For v2 objects,
nopack is initialised from sd_attrs */
REISERFS_I(inode)->i_flags &= ~i_nopack_mask;
+ REISERFS_I(inode)->i_attrs = 0;
} else {
// new stat data found, but object may have old items
// (directories and symlinks)
diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c
index ef5e541..acafe32 100644
--- a/fs/reiserfs/super.c
+++ b/fs/reiserfs/super.c
@@ -1124,7 +1124,9 @@ static void handle_attrs(struct super_bl
"reiserfs: cannot support attributes until flag is set in super-block");
REISERFS_SB(s)->s_mount_opt &= ~(1 << REISERFS_ATTRS);
}
- } else if (le32_to_cpu(rs->s_flags) & reiserfs_attrs_cleared) {
+ } else if ((le32_to_cpu(rs->s_flags) & reiserfs_attrs_cleared) &&
+ (get_inode_sd_version(s->s_root->d_inode) == STAT_DATA_V2)) {
+ /* Enable attrs by default on v3.6-native file systems */
REISERFS_SB(s)->s_mount_opt |= (1 << REISERFS_ATTRS);
}
}

But this patch has another problem - it forces the "attrs" option on
for such filesystems, leaving no way to turn it off - the "noattrs"
option is ignored. This does not look good.


Attachments:
(No filename) (3.51 kB)
(No filename) (189.00 B)
Download all attachments

2006-02-12 23:03:21

by Bernd Schubert

[permalink] [raw]
Subject: Re: 2.6.15 Bug? New security model?

> diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c
> index ef5e541..acafe32 100644
> --- a/fs/reiserfs/super.c
> +++ b/fs/reiserfs/super.c
> @@ -1124,7 +1124,9 @@ static void handle_attrs(struct super_bl
> "reiserfs: cannot support attributes until flag is set in
> super-block"); REISERFS_SB(s)->s_mount_opt &= ~(1 << REISERFS_ATTRS);
> }
> - } else if (le32_to_cpu(rs->s_flags) & reiserfs_attrs_cleared) {
> + } else if ((le32_to_cpu(rs->s_flags) & reiserfs_attrs_cleared) &&
> + (get_inode_sd_version(s->s_root->d_inode) == STAT_DATA_V2)) {
> + /* Enable attrs by default on v3.6-native file systems */
> REISERFS_SB(s)->s_mount_opt |= (1 << REISERFS_ATTRS);
> }
> }

I'm afraid that still doesn't solve the problem for me, I added two printk to
be sure whats going on - get_inode_sd_version(s->s_root->d_inode) returns
STAT_DATA_V2 for all of my partitions.

2006-02-12 23:10:48

by Bernd Schubert

[permalink] [raw]
Subject: Re: 2.6.15 Bug? New security model?

> This is essentially code-introduced corruption. The patch to fix it in
> future versions is easy enough, but you'll need to run reiserfsck
> --clean-attributes <device> on any affected file systems.
>
> Bernd - If you haven't already run reiserfsck --clean-attributes on the fs,
> can you please test the attached patch? It adds a check to make sure the
> file system has always been v3.6 before enabling the attributes by default.

With attrs defaulting to on I don't think 'reiserfsck --clean-attributes' can
be the solution. There are just too many filesystems that are affected. So
for now I also won't clean up the attributes ;)
Since Sergey already proved that directories always have the old format, I
didn't test your patch, but tried Sergeys patch.

Thanks for all of your help,
Bernd


2006-02-13 05:47:36

by Hans Reiser

[permalink] [raw]
Subject: Re: 2.6.15 Bug? New security model?

This is an xattr bug, and I'll let jeff answer it.

Hans

Sergey Vlasov wrote:

>On Wed, 8 Feb 2006 14:11:24 -0800 Chris Wright wrote:
>
>
>
>>* Bernd Schubert ([email protected]) wrote:
>>
>>
>>>Er, you mean /proc/fs/reiserfs/{partition}/on-disk-super?
>>>
>>>
>>Yup.
>>
>>
>>
>>>bernd@bathl ~>grep attrs_cleared /proc/fs/reiserfs/hda6/on-disk-super
>>>flags: 1[attrs_cleared]
>>>
>>>
>>>>2) does mount -o attrs ... make a difference?
>>>>
>>>>
>>>Yes, 2.6.13 now makes the same trouble. No difference with 2.6.15.3.
>>>I played with mount -o noattrs, this makes no difference with 2.6.13, but has
>>>some effects to 2.6.15.3. Creating files in /var/run is possible again,
>>>lsattr gives "lsattr: Inappropriate ioctl for device While reading flags
>>>on /var/run", but deleting files in /var/run is still impossible (still
>>>rather bad for the init-scripts).
>>>
>>>
>
>Is the filesystem in question old - could it be created initially in the
>reiserfs v3.5 format (used with 2.2.x kernels) and later converted to
>v3.6 (by mounting with the "conv" option)?
>
>
>
>>Yes, that's what I thought. There's still some backward logic in there.
>>noattrs vs. attrs triggers whether the code path that's patched in
>>2.6.15.3 is taken. I'll dig a bit more, but hopefully the reiserfs folks
>>can fix this for us.
>>
>>
>
>Here is a simple test case which reproduces the problem with a
>filesystem converted from v3.5:
>
># dd if=/dev/zero of=tmp.img bs=1M count=100
># mkreiserfs --format 3.5 -f tmp.img
># mount -t reiserfs -o loop,conv tmp.img /mnt/disk/
># umount /mnt/disk/
># reiserfsck --clean-attributes tmp.img
># mount -t reiserfs -o loop tmp.img /mnt/disk/
>
>At this point, I get obviously wrong attributes on /mnt/disk:
>
># lsattr -d /mnt/disk/
>-----a--c---- /mnt/disk/
>
>BTW, this breaks even with kernels earlier than 2.6.15.2, if you also
>add the "attrs" options to the last mount command.
>
>Apparently the reiserfs attrs code has been broken for such converted
>filesystems for some time, but it could be enabled only with the "attrs"
>option, so people were not hitting this. However, the following patch:
>
>http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=2949ccf9379678df66ecf2ca70ed4656159eacdd
>
>changed the logic to enable the "attrs" option on all filesystems which
>have the reiserfs_attrs_cleared flag. But that patch was broken - it
>did not really set the option properly, so the attrs-related breakage
>did not became visible until yet another patch:
>
>http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=d35c602870ece3166cff3d25fbc687a7f707acf3
>
>which later made into 2.6.15.2, and caused problems for some people.
>
>I have noticed that fs/reiserfs/inode.c:init_inode() does not initialize
>REISERFS_I(inode)->i_attrs and inode->i_flags (as done by
>sd_attrs_to_i_attrs()) in the branch for v1 stat data; maybe this causes
>the problem?
>
>

2006-02-13 15:21:33

by Jeff Mahoney

[permalink] [raw]
Subject: Re: 2.6.15 Bug? New security model?

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hans Reiser wrote:
> This is an xattr bug, and I'll let jeff answer it.

Hans -

This bug is about inode attributes (the chattr type), not extended
attributes (the setfacl/setfattr type). Regardless, it's the root cause
of the random attributes we were seeing when the REISERFS_ATTRS
enable-by-default problem was corrected. Thanks to some other people on
the list, I was able to post some patches to address it yesterday evening.

- -Jeff

- --
Jeff Mahoney
SUSE Labs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFD8KS5LPWxlyuTD7IRApKRAJ0SdbS+/KzO8Kn7RMfVQ2KfwNSg/gCfXIdE
KlxigEBlCZixvy7PObKebE0=
=3wk5
-----END PGP SIGNATURE-----

2006-02-13 16:39:26

by Hans Reiser

[permalink] [raw]
Subject: Re: 2.6.15 Bug? New security model?

Ah, thanks for the patches Jeff, apologies for misreading it.

Hans

Jeff Mahoney wrote:

> Hans Reiser wrote:
>
> >This is an xattr bug, and I'll let jeff answer it.
>
>
> Hans -
>
> This bug is about inode attributes (the chattr type), not extended
> attributes (the setfacl/setfattr type). Regardless, it's the root cause
> of the random attributes we were seeing when the REISERFS_ATTRS
> enable-by-default problem was corrected. Thanks to some other people on
> the list, I was able to post some patches to address it yesterday evening.
>
> -Jeff
>
> --
> Jeff Mahoney
> SUSE Labs

2006-02-13 17:53:15

by Sergey Vlasov

[permalink] [raw]
Subject: Re: 2.6.15 Bug? New security model?

On Mon, Feb 13, 2006 at 12:03:15AM +0100, Bernd Schubert wrote:
> > diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c
> > index ef5e541..acafe32 100644
> > --- a/fs/reiserfs/super.c
> > +++ b/fs/reiserfs/super.c
> > @@ -1124,7 +1124,9 @@ static void handle_attrs(struct super_bl
> > "reiserfs: cannot support attributes until flag is set in
> > super-block"); REISERFS_SB(s)->s_mount_opt &= ~(1 << REISERFS_ATTRS);
> > }
> > - } else if (le32_to_cpu(rs->s_flags) & reiserfs_attrs_cleared) {
> > + } else if ((le32_to_cpu(rs->s_flags) & reiserfs_attrs_cleared) &&
> > + (get_inode_sd_version(s->s_root->d_inode) == STAT_DATA_V2)) {
> > + /* Enable attrs by default on v3.6-native file systems */
> > REISERFS_SB(s)->s_mount_opt |= (1 << REISERFS_ATTRS);
> > }
> > }
>
> I'm afraid that still doesn't solve the problem for me, I added two printk to
> be sure whats going on - get_inode_sd_version(s->s_root->d_inode) returns
> STAT_DATA_V2 for all of my partitions.

Too bad. Looks like autoenabling of the "attrs" options won't fly,
and the only safe solution is to revert those patches and require
explicit "attrs" option.


Attachments:
(No filename) (1.12 kB)
(No filename) (189.00 B)
Download all attachments