linux-kernel - Re: [PATCH] debugfs: fix mount options not being applied

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <87plcyixy9.fsf@posteo.net>
Date: Wed, 13 Aug 2025 23:49:53 +0000
From: Charalampos Mitrodimas <charmitro@...teo.net>
To: Eric Sandeen <sandeen@...deen.net>
Cc: Christian Brauner <brauner@...nel.org>,  Eric Sandeen
 <sandeen@...hat.com>,  Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
  "Rafael J. Wysocki" <rafael@...nel.org>,  Danilo Krummrich
 <dakr@...nel.org>,  David Howells <dhowells@...hat.com>,
  linux-kernel@...r.kernel.org,  linux-fsdevel
 <linux-fsdevel@...r.kernel.org>
Subject: Re: [PATCH] debugfs: fix mount options not being applied

Eric Sandeen <sandeen@...deen.net> writes:

> On 8/8/25 9:13 AM, Christian Brauner wrote:
>> On Wed, Aug 06, 2025 at 11:33:11AM -0500, Eric Sandeen wrote:
>>> On 8/5/25 12:22 PM, Charalampos Mitrodimas wrote:
>
> ...
>
>>>> Hi, thanks for the review, and yes you're right.
>>>>
>>>> Maybe a potential systemic fix would be to make get_tree_single() always
>>>> call fc->ops->reconfigure() after vfs_get_super() when reusing an
>>>> existing superblock, fixing all affected filesystems at once.
>>>
>>> Yep, I'm looking into that. mount_single used to do this, and IIRC we discussed
>>> it before but for some reason opted not to. It seems a bit trickier than I first
>>> expected, but I might just be dense. ;)
>> 
>> If we can make it work generically, we should. I too don't remember what
>> the reasons were for not doing it that way.
>
> Sorry for the long delay here. Talked to dhowells about this and his
> POV (which is convincing, I think) is that even though mount_single used to
> call do_remount_sb for an extant single sb, this was probably Bad(tm).
> Bad, IIUC, because it's not a given that options are safe to be changed
> in this way, and that policy really should be up to each individual
> filesystem.
>
> So while we still need to audit and fix any get_tree_single()
> filesystems that changed behavior with the new mount api, may as well
> fix up debugfs for now since the bug was reported.

What if we add a new flag (.fs_flags), say FS_SINGLE_RECONF, to
file_system_type that makes get_tree_single() automatically call
reconfigure() when reusing an existing superblock? Filesystems could
then just opt-in by adding it to .fs_flags.

>
> Charalampos - 
>
> Your patch oopses on boot for me - I think that when you added
>
> 	sb->s_fs_info = fc->s_fs_info;

Yes, did take notice of this yesterday when I revisited it.

>
> in debugfs_fill_super, you're actually NULLing out the one in the sb,
> because sget_fc has already transferred fc->s_fs_info to sb->s_fs_info,
> and NULLed fc->s_fs_info prior to this. Then when we get to
> _debugfs_apply_options, *fsi = sb->s_fs_info; is also NULL so using it
> there oopses.
>
> If you want to send a V2 with fixed up stable cc: I'd suggest following the
> pattern of what was done for tracefs in e4d32142d1de, which I think works
> OK and would at least lend some consistency, as the code is similar.
>
> If not, let me know and I'll work on an update.

As a matter of fact, I have a v2 exactly like this ready to sent. Doing
so in a bit.

>
> Thanks,
> -Eric 

Thanks!
C. Mitrodimas