Re: [PATCH v2 2/4] ntfs: add pathname access for named streams

[Namjae Jeon]

On Wed, Oct 7, 2026 at 12:38 PM CharSyam <charsyam@xxxxxxxxx> wrote:
>
> Hi Namjae,
>
> I built the patches and tested the pathname interface on an NTFS image
> in QEMU. I found two issues:
>
> 1. **A named stream can be deleted from a read-only base file.**
>
>    I created a file and a named stream, then changed the file to mode
> 0444. The NTFS `READONLY` attribute was present on disk. As an
> unprivileged user, opening the base file for writing failed with
> `EACCES`, but `unlink("base:secret")` succeeded and the stream
> disappeared.
>
>    The new `ntfs_unlink_named_stream()` path calls
> `ntfs_remove_named_stream()` directly. Unlike the ioctl path, it does
> not require a writable base-file descriptor. The shared removal helper
> checks append-only and immutable flags but does not check the NTFS
> `READONLY` attribute. I suggest rejecting removal of a stream from a
> `READONLY` base file in the shared helper, and defining the
> authorization rule for pathname stream deletion explicitly. This is a
> deletion-semantics issue; the test does not establish a privilege
> escalation, since the parent directory was writable. The MS-FSA
> `FileDispositionInformation` rules also reject deletion when the file
> has `FILE_ATTRIBUTE_READONLY`.
> 2. **Repeated reads update the base file's atime under `relatime`.**
>
>    I read the same named stream twice, two seconds apart. Both reads
> changed the base file's atime. A regular file used as a control
> changed atime only on the first read; with `noatime`, neither stream
> read changed it.
>
>    VFS checks the stream inode's atime before calling `->update_time`.
> In `ntfs_stream_update_time_common()`, validation copies the _old_
> base atime to the stream inode, then `generic_update_time(base_vi,
> ...)` updates only the base inode. The stream inode remains stale, so
> the next read triggers another update. I suggest refreshing the stream
> inode after a successful base update:
>
>    ```
>    err = ntfs_stream_inode_validate(inode);
>    if (!err) {
>            err = generic_update_time(base_vi, type, flags);
>            if (!err)
>                    ntfs_stream_inode_refresh(inode);
>    }
>    ```
>
>    I built and tested this change in QEMU. After the first read, the
> second read no longer changed atime. This avoids unnecessary metadata
> updates; the test does not imply that every read caused a physical
> disk write.
Okay, I will fix it in v3.
Thanks for the review!