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
