2004-10-02 22:50:12

by Dino Klein

[permalink] [raw]
Subject: PROBLEM: 2.6.9-rc3 Bug in NTFS code

below is what I had in the logs when attempting to umount a readonly NTFS.

------------[ cut here ]------------
kernel BUG at fs/ntfs/inode.c:354!
invalid operand: 0000 [#1]
SMP
Modules linked in: ntfs ohci1394 ieee1394 via_agp snd_emu10k1 snd_rawmidi
snd_pcm snd_timer snd_seq_device snd_ac97_codec snd_page_alloc snd_util_mem
snd_hwdep snd soundcore hpt366 ipv6 parport_pc lp parport autofs4 sunrpc
3c59x ipt_REJECT ipt_state ip_conntrack iptable_filter ip_tables floppy sg
scsi_mod microcode tsdev joydev usbhid dm_mod uhci_hcd ohci_hcd ehci_hcd
usbcore button battery asus_acpi ac ext3 jbd
CPU: 0
EIP: 0060:[<f8c7f456>] Not tainted VLI
EFLAGS: 00010286 (2.6.9-rc3)
EIP is at ntfs_destroy_extent_inode+0x26/0x30 [ntfs]
eax: c1ef89c0 ebx: f6cdfc40 ecx: 00000000 edx: f5a3c9b0
esi: 00000002 edi: f73b5e00 ebp: f73b5e50 esp: f5affef8
ds: 007b es: 007b ss: 0068
Process umount (pid: 3161, threadinfo=f5afe000 task=f72d4bf0)
Stack: f8c81cbf f6cdfcec f73b5e00 c0174734 f6cdfcec c017561c f6cdfcec
f594aee0
c01756b3 f594af60 f8c84912 f73b5e00 f5afe000 00000000 c0161d46
ffffffff
f8c8cd20 f73b5e00 f7f94200 f8c8cda0 f5afe000 c01627e7 f73b5e40
f73b5e00
Call Trace:
[<f8c81cbf>] ntfs_clear_big_inode+0x6f/0x80 [ntfs]
[<c0174734>] clear_inode+0xf4/0x130
[<c017561c>] generic_forget_inode+0xec/0x110
[<c01756b3>] iput+0x53/0x70
[<f8c84912>] ntfs_put_super+0xf2/0x2c0 [ntfs]
[<c0161d46>] generic_shutdown_super+0x176/0x190
[<c01627e7>] kill_block_super+0x17/0x40
[<c0161afe>] deactivate_super+0x6e/0x90
[<c0177a0b>] sys_umount+0x3b/0x90
[<c01500ec>] do_munmap+0x12c/0x170
[<c0177a75>] sys_oldumount+0x15/0x20
[<c010606d>] sysenter_past_esp+0x52/0x71
Code: 26 00 00 00 00 89 c2 8b 40 58 85 c0 75 1d f0 ff 4a 1c 0f 94 c0 84 c0
75 08 0f 0b 64 01 1d 6c c8 f8 a1 3c d4 c8 f8 e9 aa 7b 4c c7 <0f> 0b 62 01 1d
6c c8 f8 eb d9 c7 42 1c 01 00 00 00 8d 4a 3c c7

_________________________________________________________________
Check out Election 2004 for up-to-date election news, plus voter tools and
more! http://special.msn.com/msn/election2004.armx


2004-10-03 00:27:48

by Anton Altaparmakov

[permalink] [raw]
Subject: Re: PROBLEM: 2.6.9-rc3 Bug in NTFS code

On Sat, 2 Oct 2004, Dino Klein wrote:
> below is what I had in the logs when attempting to umount a readonly NTFS.
[snip]

I suspect that this is caused by a bug I introduced in 2.1.18 release and
that I already fixed in my development tree. I unfortunately completely
forgot that the code containing the bug was already in the mainstream
kernels so I didn't think of submitting the fix straight away. )))-:

Linus, please do a

bk pull bk://linux-ntfs.bkbits.net/ntfs-2.6

to apply the fix which is also shown below in diff style patch. Thanks!

Dino, could you apply the below patch and just check that the bug goes
away (if you prefer just add by hand the line "ctx->al_entry = NULL;" to
fs/ntfs/attrib.c as shown at the bottom of the patch). It is always
possible I introduced a different bug I haven't found yet as well so it
would be good to know that this fix fixes the problem you are
experiencing... Thanks!

Best regards,

Anton
--
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/, http://www-stu.christs.cam.ac.uk/~aia21/

This will update the following files:

Documentation/filesystems/ntfs.txt | 2 ++
fs/ntfs/ChangeLog | 7 +++++++
fs/ntfs/Makefile | 2 +-
fs/ntfs/attrib.c | 5 +++++
4 files changed, 15 insertions(+), 1 deletion(-)

through these ChangeSets:

<[email protected]> (04/10/03 1.2031.1.1)
NTFS: Fix stupid bug in fs/ntfs/attrib.c::ntfs_attr_reinit_search_ctx() where
we did not clear ctx->al_entry but it was still set due to changes in
ntfs_attr_lookup() and ntfs_external_attr_find() in particular.

Signed-off-by: Anton Altaparmakov <[email protected]>

===================================================================

diff -Nru a/Documentation/filesystems/ntfs.txt b/Documentation/filesystems/ntfs.txt
--- a/Documentation/filesystems/ntfs.txt 2004-10-03 01:16:22 +01:00
+++ b/Documentation/filesystems/ntfs.txt 2004-10-03 01:16:22 +01:00
@@ -277,6 +277,8 @@

Note, a technical ChangeLog aimed at kernel hackers is in fs/ntfs/ChangeLog.

+2.1.20:
+ - Fix a stupid bug introduced in 2.1.18 release.
2.1.19:
- Minor bugfix in handling of the default upcase table.
- Many internal cleanups and improvements. Many thanks to Linus
diff -Nru a/fs/ntfs/ChangeLog b/fs/ntfs/ChangeLog
--- a/fs/ntfs/ChangeLog 2004-10-03 01:16:22 +01:00
+++ b/fs/ntfs/ChangeLog 2004-10-03 01:16:22 +01:00
@@ -21,6 +21,13 @@
- Enable the code for setting the NT4 compatibility flag when we start
making NTFS 1.2 specific modifications.

+2.1.20 - Fix a stupid bug in ntfs_attr_reinit_search_ctx().
+
+ - Fix stupid bug in fs/ntfs/attrib.c::ntfs_attr_reinit_search_ctx()
+ where we did not clear ctx->al_entry but it was still set due to
+ changes in ntfs_attr_lookup() and ntfs_external_attr_find() in
+ particular.
+
2.1.19 - Many cleanups, improvements, and a minor bug fix.

- Update ->setattr (fs/ntfs/inode.c::ntfs_setattr()) to refuse to
diff -Nru a/fs/ntfs/Makefile b/fs/ntfs/Makefile
--- a/fs/ntfs/Makefile 2004-10-03 01:16:22 +01:00
+++ b/fs/ntfs/Makefile 2004-10-03 01:16:22 +01:00
@@ -6,7 +6,7 @@
index.o inode.o mft.o mst.o namei.o super.o sysctl.o unistr.o \
upcase.o

-EXTRA_CFLAGS = -DNTFS_VERSION=\"2.1.19\"
+EXTRA_CFLAGS = -DNTFS_VERSION=\"2.1.20\"

ifeq ($(CONFIG_NTFS_DEBUG),y)
EXTRA_CFLAGS += -DDEBUG
diff -Nru a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
--- a/fs/ntfs/attrib.c 2004-10-03 01:16:22 +01:00
+++ b/fs/ntfs/attrib.c 2004-10-03 01:16:22 +01:00
@@ -1861,6 +1861,11 @@
/* Sanity checks are performed elsewhere. */
ctx->attr = (ATTR_RECORD*)((u8*)ctx->mrec +
le16_to_cpu(ctx->mrec->attrs_offset));
+ /*
+ * This needs resetting due to ntfs_external_attr_find() which
+ * can leave it set despite having zeroed ctx->base_ntfs_ino.
+ */
+ ctx->al_entry = NULL;
return;
} /* Attribute list. */
if (ctx->ntfs_ino != ctx->base_ntfs_ino)

2004-10-03 07:25:28

by Anton Altaparmakov

[permalink] [raw]
Subject: Re: PROBLEM: 2.6.9-rc3 Bug in NTFS code

On Sun, 3 Oct 2004, Anton Altaparmakov wrote:
> On Sat, 2 Oct 2004, Dino Klein wrote:
> > below is what I had in the logs when attempting to umount a readonly NTFS.
> [snip]
>
> I suspect that this is caused by a bug I introduced in 2.1.18 release and
> that I already fixed in my development tree. I unfortunately completely
> forgot that the code containing the bug was already in the mainstream
> kernels so I didn't think of submitting the fix straight away. )))-:

Ouch. Having slept over it I awoke this morning with the realization that
there is in fact another bug, which would definitely cause exactly the BUG
check to trigger that Dino reported. )-: Sorry about that. Just goes to
show that when I was porting my code from libntfs to the kernel and
thinking "wow, this is really too easy", I was actually missing the
subtleties involved so I broke it. )-:

Linus, please do a

bk pull bk://linux-ntfs.bkbits.net/ntfs-2.6

to apply the fix which is also shown below in diff style patch. Thanks!

Dino, could you apply the below patch and just check that the bug goes
away in case I have missed yet another problem... I am pretty confident
that this is it this time but you never know. (You can just add the lines
"if (ni != base_ni) unmap_extent_mft_record(ni);" to fs/ntfs/attrib.c as
shown in the patch.) Thanks a lot in advance!

Note you also want the fix from my previous email as the two bugs/fixes
are related...

Best regards,

Anton
--
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/

This will update the following files:

Documentation/filesystems/ntfs.txt | 2 +-
fs/ntfs/ChangeLog | 4 ++++
fs/ntfs/attrib.c | 4 +++-
3 files changed, 8 insertions(+), 2 deletions(-)

through these ChangeSets:

<[email protected]> (04/10/03 1.2034.1.1)
NTFS: Fix another stupid bug in fs/ntfs/attrib.c::ntfs_external_attr_find()
where we forgot to unmap the extent mft record when we had finished
enumerating an attribute which caused a bug check to trigger when the
VFS calls ->clear_inode.

Signed-off-by: Anton Altaparmakov <[email protected]>

===================================================================

diff -Nru a/Documentation/filesystems/ntfs.txt b/Documentation/filesystems/ntfs.txt
--- a/Documentation/filesystems/ntfs.txt 2004-10-03 08:17:35 +01:00
+++ b/Documentation/filesystems/ntfs.txt 2004-10-03 08:17:35 +01:00
@@ -278,7 +278,7 @@
Note, a technical ChangeLog aimed at kernel hackers is in fs/ntfs/ChangeLog.

2.1.20:
- - Fix a stupid bug introduced in 2.1.18 release.
+ - Fix two stupid bugs introduced in 2.1.18 release.
2.1.19:
- Minor bugfix in handling of the default upcase table.
- Many internal cleanups and improvements. Many thanks to Linus
diff -Nru a/fs/ntfs/ChangeLog b/fs/ntfs/ChangeLog
--- a/fs/ntfs/ChangeLog 2004-10-03 08:17:35 +01:00
+++ b/fs/ntfs/ChangeLog 2004-10-03 08:17:35 +01:00
@@ -27,6 +27,10 @@
where we did not clear ctx->al_entry but it was still set due to
changes in ntfs_attr_lookup() and ntfs_external_attr_find() in
particular.
+ - Fix another stupid bug in fs/ntfs/attrib.c::ntfs_external_attr_find()
+ where we forgot to unmap the extent mft record when we had finished
+ enumerating an attribute which caused a bug check to trigger when the
+ VFS calls ->clear_inode.

2.1.19 - Many cleanups, improvements, and a minor bug fix.

diff -Nru a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
--- a/fs/ntfs/attrib.c 2004-10-03 08:17:35 +01:00
+++ b/fs/ntfs/attrib.c 2004-10-03 08:17:35 +01:00
@@ -1738,11 +1738,13 @@
* correctly yet as we do not know what @ctx->attr will be set to by
* the call to ntfs_attr_find() below.
*/
+ if (ni != base_ni)
+ unmap_extent_mft_record(ni);
ctx->mrec = ctx->base_mrec;
ctx->attr = (ATTR_RECORD*)((u8*)ctx->mrec +
le16_to_cpu(ctx->mrec->attrs_offset));
ctx->is_first = TRUE;
- ctx->ntfs_ino = ctx->base_ntfs_ino;
+ ctx->ntfs_ino = base_ni;
ctx->base_ntfs_ino = NULL;
ctx->base_mrec = NULL;
ctx->base_attr = NULL;