Re: [GIT PULL] ntfs updates for 7.1-rc1(v2)
From: Namjae Jeon
Date: Fri Apr 17 2026 - 21:31:37 EST
On Sat, Apr 18, 2026 at 9:01 AM Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Thu, 16 Apr 2026 at 22:13, Namjae Jeon <linkinjeon@xxxxxxxxxx> wrote:
> >
> > In this v2, I have dropped the two build-fix patches that were causing
> > issues in the previous PR. here is the patches that cause semantic
> > conflicts with this series in linux-next:
>
> Ok, I've pulled this, and it's going through some test runs.
>
> However, when merging it, I think I've already found a bug.
>
> The merge resolution in ntfs_file_mmap_prepare() was not complicated,
> but I think the code is simply *buggy*.
>
> Checking whether the VMA_WRITE_BIT is set seems nonsensical. Yes, a
> shared writable mmap has that bit set - but so does a *private*
> writable mmap, which is a lot more common and doesn't actually write
> to the filesystem (it only reads, and then does a copy-on-write).
>
> But on the other hand, checking for VMA_WRITE_BIT is wrong for
> *another* reason: maybe the mapping isn't writable, but if it's a
> shared mapping of a file that was opened with write permissions, it
> can *become* writable with a simple mprotect().
>
> In other words,. checking VMA_WRITE_BIT is just nonsensical. It is
> meaningless. It has literally nothing to do with "will this mapping
> write to this file", but that *seems* to be how you use it.
>
> VMA_WRITE_BIT is about whether the resulting mapping is writable, but
> it's about whether it's writable as a *user mapping*, not whether it's
> writable as a *filesystem write*.
>
> And the two are very different things.
>
> A mapping will write to the filesystem only if it is both
>
> (a) VM_SHARED (not VM_PRIVATE)
>
> (b) when the file descriptor was opened for writing, which sets VM_MAYWRITE.
>
> and so the VM_WRITE bit is almost entirely irrelevant (I say "almost",
> because in theory I guess you could check for VM_WRITE instead of
> VM_MAYWRITE, but then you'd *also* have to catch mprotect() changing
> the actual writability - but no filesystem has ever taken that
> approach afaik).
>
> I'm not going to fix that as part of the merge, and I think I'll leave
> the merge in place, but this needs looking at. Because that code
> _looks_ nonsensical to me.
>
> The proper test for an actual writable shared mapping - that some
> filesystems seem to do - is either
>
> if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_MAYWRITE))
>
> in the mmap function (like fuse does) or
>
> if (vma_desc_test_all(desc, VMA_SHARED_BIT, VMA_MAYWRITE_BIT))
>
> (in the mmap_prepare function like erofs and zonefs do).
>
> But I don't know this code, and I'm not going to make that change
> blindly. Maybe this code does something else than what I think it
> does.
Thank you very much for catching that bug. I will check the logic you
mentioned regarding VMA flags and fix it in the next PR. And I truly
appreciate you merging the NTFS resurrection series!