Re: [PATCH 2/2] ntfs3: remove warning

[Johan Hovold <johan@kernel.org>]

On Tue, Apr 16, 2024 at 12:38:56PM +0200, Christian Brauner wrote:
> On Mon, Apr 15, 2024 at 06:06:03PM +0200, Johan Hovold wrote:

> > Ah, right, I forgot about CONFIG_NTFS_RW as I've never enabled it.
> > 
> > Judging from the now removed Kconfig entry perhaps not that many people
> > did:
> > 
> > 	The only supported operation is overwriting existing files,
> > 	without changing the file length.  No file or directory
> > 	creation, deletion or renaming is possible. 
> > 
> > but I guess it still makes my argument above mostly moot.
> > 
> > At least if we disable write support in ntfs3 by default for now...
> 
> I think we can disable write support in ntfs3 for now. I've picked up
> the patch to make ntfs3 serve I sent some time ago that Johan tested
> now.

Note that I actually meant that write support should be disabled
completely in ntfs3 for now.

After this first encounter I have zero confidence in that driver and
pushing people towards using it (by removing the old, read-only one) is
just gonna result in further corrupted filesystems. At least make sure
it can't modify anything by default and mark write-support as
experimental and broken or something as that's apparently what it is.

> The only thing left is to disable write support for ntfs3 as legacy ntfs
> driver for now. I took a stab at this. The following two patches
> I'm appending _should_ be enough iiuc. Johan, please take a look and
> please test.

I skimmed them and gave them a quick spin. It seems that not specifying
either "ro" or "rw" in fstab now results in a ro mount, but I can still
specify "rw" explicitly (in fstab or command line) and end up with:

	/dev/nvme0n1p3 on /mnt/windows type ntfs (rw,relatime,uid=0,gid=0,iocharset=iso8859-1)

For obvious reasons, I did not dare listing the root directory or write
anything, but it looks like it's not read-only.

Using just my naive temporary hack from yesterday:

diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index 8d2e51bae2cb..26be6c6d1032 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -1177,6 +1177,9 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
        sb->s_xattr = ntfs_xattr_handlers;
        sb->s_d_op = options->nocase ? &ntfs_dentry_ops : NULL;

+       ntfs_warn(sb, "ntfs3 driver is broken, mounting read only");
+       sb->s_flags |= SB_RDONLY;
+
        options->nls = ntfs_load_nls(options->nls_name);
        if (IS_ERR(options->nls)) {
                options->nls = NULL;

seems to prevent also explicit rw mounts (but judging from your patches
it is not necessarily sufficient to prevent all modifications).

Johan