2003-08-31 04:13:06

by Valdis Klētnieks

[permalink] [raw]
Subject: 2.6.0-test4-mm3.1 oops with ext3 extended attributes on R/O filesystem

Working on installing SELINUX, and I get to the part where all the file labels
get added. Unfortunately, I had some file systems mounted R/O (intentionally,
forgot to mount them R/W for this). The ext3 code upchucked while trying
to set extended attributes on the filesystem it couldn't write to.

In addition to the OOPS, it seemed to leave a lock dangling - even after remounting
the filesystem R/W and trying again, it just sat there.

Proper action would probably be to -EPERM rather than OOPS...

Aug 30 17:24:04 turing-police kernel: Unable to handle kernel paging request at virtual address ffffffe2
Aug 30 17:24:04 turing-police kernel: printing eip:
Aug 30 17:24:04 turing-police kernel: c0184c46
Aug 30 17:24:04 turing-police kernel: *pde = 00001067
Aug 30 17:24:04 turing-police kernel: *pte = 00000000
Aug 30 17:24:04 turing-police kernel: Oops: 0000 [#1]
Aug 30 17:24:04 turing-police kernel: PREEMPT
Aug 30 17:24:04 turing-police kernel: CPU: 0
Aug 30 17:24:04 turing-police kernel: EIP: 0060:[__ext3_journal_stop+8/60] Not tainted VLI
Aug 30 17:24:04 turing-police kernel: EFLAGS: 00010216
Aug 30 17:24:04 turing-police kernel: EIP is at __ext3_journal_stop+0x8/0x3c
Aug 30 17:24:04 turing-police kernel: eax: ffffffe2 ebx: ffffffe2 ecx: cf429990 edx: cfd75000
Aug 30 17:24:04 turing-police kernel: esi: ffffffe2 edi: cf429990 ebp: cf30fdc8 esp: cf30fdc0
Aug 30 17:24:04 turing-police kernel: ds: 007b es: 007b ss: 0068
Aug 30 17:24:04 turing-police kernel: Process setfiles (pid: 980, threadinfo=cf30e000 task=cf2786b0)
Aug 30 17:24:04 turing-police kernel: Stack: ffffffe2 ffffffe2 cf30fde4 c0188b4e c0355ca1 ffffffe2 c03d022c cf30fe66
Aug 30 17:24:04 turing-police kernel: c03693b4 cf30fe0c c018a0a9 cf429990 00000006 cf30fe65 cef53d60 00000019
Aug 30 17:24:04 turing-police kernel: 00000000 00000005 cef53d60 cf30fe3c c0187a79 cf429990 cf30fe65 cef53d60
Aug 30 17:24:04 turing-police kernel: Call Trace:
Aug 30 17:24:04 turing-police kernel: [ext3_xattr_set+68/83] ext3_xattr_set+0x44/0x53
Aug 30 17:24:04 turing-police kernel: [ext3_xattr_security_set+60/71] ext3_xattr_security_set+0x3c/0x47
Aug 30 17:24:05 turing-police kernel: [ext3_setxattr+175/183] ext3_setxattr+0xaf/0xb7
Aug 30 17:24:05 turing-police kernel: [setxattr+373/444] setxattr+0x175/0x1bc
Aug 30 17:24:05 turing-police kernel: [inode_has_perm+93/101] inode_has_perm+0x5d/0x65
Aug 30 17:24:05 turing-police kernel: [dput+28/508] dput+0x1c/0x1fc
Aug 30 17:24:05 turing-police kernel: [dput+28/508] dput+0x1c/0x1fc
Aug 30 17:24:05 turing-police kernel: [link_path_walk+1754/1932] link_path_walk+0x6da/0x78c
Aug 30 17:24:05 turing-police kernel: [sys_lsetxattr+49/66] sys_lsetxattr+0x31/0x42
Aug 30 17:24:05 turing-police kernel: [sysenter_past_esp+67/101] sysenter_past_esp+0x43/0x65
Aug 30 17:24:05 turing-police kernel:
Aug 30 17:24:05 turing-police kernel: Code: 68 a0 5a 35 c0 52 e8 02 02 00 00 83 c4 0c eb 0c 89 4d 0c 89 45 08 c9 e9 5e 58 00 00 b8 e2 ff ff ff c9 c3 55 89 e5 56 53 8b 45 0c <8b> 10 8b 58 0c 8b 12 8b b2 dc 00 00 00 50 e8 b0 68 00 00 85 db



Attachments:
(No filename) (226.00 B)

2003-08-31 04:44:16

by Andrew Morton

[permalink] [raw]
Subject: Re: 2.6.0-test4-mm3.1 oops with ext3 extended attributes on R/O filesystem

[email protected] wrote:
>
> Working on installing SELINUX, and I get to the part where all the file labels
> get added. Unfortunately, I had some file systems mounted R/O (intentionally,
> forgot to mount them R/W for this). The ext3 code upchucked while trying
> to set extended attributes on the filesystem it couldn't write to.

Thanks. It's a very straightforward bug; I'll fix it with the below patch.

A wider question is whether we should have got this far into the filesystem
code if the fs is mounted read-only. A check right up at the VFS
setxattr() level might make sense.

Regardless of that, this fix is needed because journal_start() could fail
for other reasons.


diff -puN fs/ext3/xattr.c~ext3-xattr-oops-fix fs/ext3/xattr.c
--- 25/fs/ext3/xattr.c~ext3-xattr-oops-fix 2003-08-30 21:41:24.000000000 -0700
+++ 25-akpm/fs/ext3/xattr.c 2003-08-30 21:42:41.000000000 -0700
@@ -873,17 +873,22 @@ ext3_xattr_set(struct inode *inode, int
const void *value, size_t value_len, int flags)
{
handle_t *handle;
- int error, error2;
+ int error;

handle = ext3_journal_start(inode, EXT3_DATA_TRANS_BLOCKS);
- if (IS_ERR(handle))
+ if (IS_ERR(handle)) {
error = PTR_ERR(handle);
- else
+ } else {
+ int error2;
+
error = ext3_xattr_set_handle(handle, inode, name_index, name,
value, value_len, flags);
- error2 = ext3_journal_stop(handle);
+ error2 = ext3_journal_stop(handle);
+ if (error == 0)
+ error = error2;
+ }

- return error ? error : error2;
+ return error;
}

/*

_

2003-08-31 11:16:14

by Andreas Gruenbacher

[permalink] [raw]
Subject: Re: 2.6.0-test4-mm3.1 oops with ext3 extended attributes on R/O filesystem

Hi,

On Sun, 2003-08-31 at 06:47, Andrew Morton wrote:
> [email protected] wrote:
> >
> > Working on installing SELINUX, and I get to the part where all the file labels
> > get added. Unfortunately, I had some file systems mounted R/O (intentionally,
> > forgot to mount them R/W for this). The ext3 code upchucked while trying
> > to set extended attributes on the filesystem it couldn't write to.
>
> Thanks. It's a very straightforward bug; I'll fix it with the below patch.

Thanks for fixing.

> A wider question is whether we should have got this far into the filesystem
> code if the fs is mounted read-only. A check right up at the VFS
> setxattr() level might make sense.

The read-only check is in ext3_xattr_set_handle(). My thinking was that
it should happen below the xattr handler level, so that attribute
classes with some sort of set behavior are possible on read-only file
systems as well. I'm not aware of an implementation that would need
this, though.

> Regardless of that, this fix is needed because journal_start() could fail
> for other reasons.

Yes.

> diff -puN fs/ext3/xattr.c~ext3-xattr-oops-fix fs/ext3/xattr.c
> --- 25/fs/ext3/xattr.c~ext3-xattr-oops-fix 2003-08-30 21:41:24.000000000 -0700
> +++ 25-akpm/fs/ext3/xattr.c 2003-08-30 21:42:41.000000000 -0700
> @@ -873,17 +873,22 @@ ext3_xattr_set(struct inode *inode, int
> const void *value, size_t value_len, int flags)
> {
> handle_t *handle;
> - int error, error2;
> + int error;
>
> handle = ext3_journal_start(inode, EXT3_DATA_TRANS_BLOCKS);
> - if (IS_ERR(handle))
> + if (IS_ERR(handle)) {
> error = PTR_ERR(handle);
> - else
> + } else {
> + int error2;
> +
> error = ext3_xattr_set_handle(handle, inode, name_index, name,
> value, value_len, flags);
> - error2 = ext3_journal_stop(handle);
> + error2 = ext3_journal_stop(handle);
> + if (error == 0)
> + error = error2;
> + }
>
> - return error ? error : error2;
> + return error;
> }
>
> /*
>
> _
--
Andreas Gruenbacher <[email protected]>
SuSE Labs, SuSE Linux AG <http://www.suse.de/>


2003-08-31 14:24:47

by Valdis Klētnieks

[permalink] [raw]
Subject: Re: 2.6.0-test4-mm3.1 oops with ext3 extended attributes on R/O filesystem

On Sat, 30 Aug 2003 21:47:51 PDT, Andrew Morton said:

> Thanks. It's a very straightforward bug; I'll fix it with the below patch.

Fix tested in -test4-mm4, program gets a proper '-EROFS', no signs of hanging
locks. Looks good from here, please push to appropriate maintainers..

> A wider question is whether we should have got this far into the filesystem
> code if the fs is mounted read-only. A check right up at the VFS
> setxattr() level might make sense.

Yes, I was a bit surprised at that as well...

> Regardless of that, this fix is needed because journal_start() could fail
> for other reasons.

Good point, and one I hadn't considered...


Attachments:
(No filename) (226.00 B)