PROBLEM: 2.6.9-rc3 Bug in NTFS code

PROBLEM: 2.6.9-rc3 Bug in NTFS code

[Dino Klein]

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

Re: PROBLEM: 2.6.9-rc3 Bug in NTFS code

[Anton Altaparmakov]

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:

<aia21@cantab.net> (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 <aia21@cantab.net>

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

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)

Re: PROBLEM: 2.6.9-rc3 Bug in NTFS code

[Anton Altaparmakov]

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:

<aia21@cantab.net> (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 <aia21@cantab.net>

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

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;

Re: PROBLEM: 2.6.9-rc3 Bug in NTFS code

[Dino Klein]

initially I applied only the second patch (since I didn't realize there were 
two different patches), and that fixed the umount problem.
afterwards I applied both, checked again, and there were no problems as 
well.

thanks.

>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:
>
><aia21@cantab.net> (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 <aia21@cantab.net>
>
>===================================================================
>
>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;

_________________________________________________________________
On the road to retirement? Check out MSN Life Events for advice on how to 
get there! http://lifeevents.msn.com/category.aspx?cid=Retirement

Re: PROBLEM: 2.6.9-rc3 Bug in NTFS code

[Anton Altaparmakov]

On Sun, 3 Oct 2004, Dino Klein wrote:
> initially I applied only the second patch (since I didn't realize there were
> two different patches), and that fixed the umount problem.
> afterwards I applied both, checked again, and there were no problems as well.
> 
> thanks.

Great, thanks for letting me know.

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/