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: <495848ab-2493-4701-b514-415377fe877b@sandeen.net>
Date: Wed, 13 Aug 2025 17:02:31 -0500
From: Eric Sandeen <sandeen@...deen.net>
To: Christian Brauner <brauner@...nel.org>
Cc: Charalampos Mitrodimas <charmitro@...teo.net>,
 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

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.

Charalampos - 

Your patch oopses on boot for me - I think that when you added

	sb->s_fs_info = fc->s_fs_info;

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.

Thanks,
-Eric