2.6.0-test4-mm3.1 oops with ext3 extended attributes on R/O filesystem

2.6.0-test4-mm3.1 oops with ext3 extended attributes on R/O filesystem

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 3099 bytes --]

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



[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: 2.6.0-test4-mm3.1 oops with ext3 extended attributes on R/O filesystem

[Andrew Morton]

Valdis.Kletnieks@vt.edu 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;
 }
 
 /*

_

Re: 2.6.0-test4-mm3.1 oops with ext3 extended attributes on R/O filesystem

[Andreas Gruenbacher]

Hi,

On Sun, 2003-08-31 at 06:47, Andrew Morton wrote:
> Valdis.Kletnieks@vt.edu 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 <agruen@suse.de>
SuSE Labs, SuSE Linux AG <http://www.suse.de/>

Re: 2.6.0-test4-mm3.1 oops with ext3 extended attributes on R/O filesystem

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 657 bytes --]

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

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]