fs/ntfs3: possible redundant MFT bitmap extent merging (ntfs_fill_super vs ntfs_read_mft)

[senjin@xxxxxxxxxxxxx]

Hi Konstantin, Hi Ruslan,

I don't have any filesystem or kernel background, so please excuse me
if I'm misreading something here. I just happened to notice this
while looking back at my own small "load ATTR_BITMAP run extents from
$MFT extension records" patch out of curiosity, and wanted to flag it
rather than assume it's already known.

Ruslan's patch (which I found here, I wasn't otherwise aware of it:
https://lkml.iu.edu/hypermail/linux/kernel/2603.3/10383.html) looks
like it fixes the same underlying issue mine does: fragmented $MFT
bitmap runs causing wnd_init()/wnd_rescan() to fail with -ENOENT. It
does this through a different code path though, and both now seem to
be present at the same time:

1. My patch extends the existing non-primary attribute segment
   handling in ntfs_read_mft() (inode.c) so that ATTR_BITMAP
   extension segments (not just the existing ATTR_DATA case) get
   routed into sbi->mft.bitmap.run. This happens as part of the
   normal $MFT inode read.

2. Ruslan's patch adds a separate loop in ntfs_fill_super()
   (super.c), just before wnd_init(), that walks the attribute list
   via ni_enum_attr_ex() looking for ATTR_BITMAP extents with
   svcn > 0 and calls run_unpack_ex() to merge them into
   sbi->mft.bitmap.run directly.

As far as I can tell, ntfs_fill_super() has to read $MFT's own inode
before it can walk its attribute list at all, so by the time Ruslan's
loop runs, ntfs_read_mft() (with my change) would already have merged
those same extension-record runs into sbi->mft.bitmap.run once
already. If I'm reading that right, every mount of a volume with a
fragmented MFT bitmap would now unpack and merge the same ATTR_BITMAP
extents twice.

I looked at run_add_entry() and it seems to merge/consolidate
overlapping ranges rather than reject them, so I don't think this
causes any actual corruption. Both passes would be unpacking the same
VCN/LCN data, so the second pass should just reconfirm the same thing
and consolidate back down harmlessly. But I could easily be missing
something, since I'm genuinely just someone who ran into this bug on
my own drive and patched the one function I could see was wrong. I
have no real experience with the rest of the driver or with how these
two changes are supposed to interact.

Apologies if this is already known or if I've misunderstood how the
two pieces fit together. Just didn't want to assume it had been
noticed. Happy to help however's useful, including sending a
follow-up patch if it turns out one of the two paths should be
removed.

Thanks,
Senjin