linux-ext4 - Re: [PATCH] ext4: reject mount options not supported when remounting in handle_mount

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20200516014958.GB3018416@iweiny-DESK2.sc.intel.com>
Date:   Fri, 15 May 2020 18:49:59 -0700
From:   Ira Weiny <ira.weiny@...el.com>
To:     "Theodore Y. Ts'o" <tytso@....edu>
Cc:     Jan Kara <jack@...e.cz>,
        Ext4 Developers List <linux-ext4@...r.kernel.org>,
        syzkaller-bugs@...glegroups.com, stable@...nel.org,
        syzbot+bca9799bf129256190da@...kaller.appspotmail.com
Subject: Re: [PATCH] ext4: reject mount options not supported when remounting
 in handle_mount_opt()

On Thu, May 14, 2020 at 10:34:09AM -0400, Theodore Y. Ts'o wrote:
> On Wed, Apr 22, 2020 at 06:10:29PM +0200, Jan Kara wrote:
> > On Wed 15-04-20 22:23:52, Ira Weiny wrote:
> > > On Wed, Apr 15, 2020 at 06:07:52PM -0400, Theodore Y. Ts'o wrote:
> > > > On Wed, Apr 15, 2020 at 01:25:37PM -0700, Ira Weiny wrote:
> > > > > This fundamentally changes the behavior from forcing the dax mode to be the
> > > > > same across the remount to only failing if we are going from non-dax to dax,
> > > > > adding -o dax on the remount?
> > > > > 
> > > > > But going from -o dax to 'not -o dax' would be ok?
> > > > > 
> > > > > FWIW after thinking about it some I _think_ it would be ok to allow the dax
> > > > > mode to change on a remount and let the inodes in memory stay in the mode they
> > > > > are at.  And newly loaded inodes would get the new mode...  Unfortunately
> > > > > without the STATX patch I have proposed the user does not have any way of
> > > > > knowing which files are in which mode.
> > > > 
> > > > We don't currently support mount -o nodax.
> > > 
> > > But we do support not supplying the option which means 'nodax' right?
> > 
> > Yeah, I second what Ira wrote. The new code does not seem to properly
> > detect a case when enabled mount option is removed for remount and thus the
> > feature would get disabled during remount as a result...
> 
> Sorry for not responding earlier.  The way ext4 remounting working is
> not supplying an mount option which toggles a switch means that we
> don't change its current setting.
> 
> For example, if you mount with, say dioread_nolock, if you don't
> specify it when remounting, the current setting of dioread_nolock
> remains the same.  If you want to change it, you need to specify the
> mount option nodioread_nolock.  The change is true for discard vs
> nodiscard, etc.
> 
> We currently don't have nodax at all, which means that once dax is
> set, there is no way to unset the dax mount option.  This was
> deliberate, because I was aware that the dax->no dax transition would
> result in badness.

At this point I'm not sure if it was working correctly before or not.

I did keep this thread in mind and did a bit more testing on the latest version
of the new DAX mount option parsing[1].

I have verified that whatever state (always, never, inode) dax was in, a
remount does not affect it and the warning is printed.

Furthermore I think the lead patches disabling verity and encryption[2] in that
series should help, if not fix, this bug.

Ted, do you think this series can make 5.8?  The prelim patches [2] could be
marked stable if you think they help without the rework of the mount option.

Thanks,
Ira

[1] https://lore.kernel.org/lkml/20200515093224.GI9569@quack2.suse.cz/
[2] https://lore.kernel.org/lkml/20200515044121.2987940-3-ira.weiny@intel.com/
    https://lore.kernel.org/lkml/20200515044121.2987940-4-ira.weiny@intel.com/