linux-kernel - Re: [PATCH 2/3 cgroup/for-5.20] cgroup: Add "no" prefixed mount options

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: <YuBIACfZDk72yjI3@slm.duckdns.org>
Date:   Tue, 26 Jul 2022 10:01:04 -1000
From:   Tejun Heo <tj@...nel.org>
To:     Michal Koutný <mkoutny@...e.com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        John Stultz <john.stultz@...aro.org>,
        Dmitry Shmidt <dimitrysh@...gle.com>,
        Oleg Nesterov <oleg@...hat.com>, linux-kernel@...r.kernel.org,
        cgroups@...r.kernel.org
Subject: Re: [PATCH 2/3 cgroup/for-5.20] cgroup: Add "no" prefixed mount
 options

Hello,

On Tue, Jul 26, 2022 at 04:32:46PM +0200, Michal Koutný wrote:
> On Thu, Jul 14, 2022 at 06:38:43PM -1000, Tejun Heo <tj@...nel.org> wrote:
> > We allow modifying these mount options via remount. Let's add "no" prefixed
> > variants so that they can be turned off too.
> 
> They can be turned off:
> 
> > // on v5.19-rc?
> > :~ # grep cg /proc/mounts
> > cgroup2 /sys/fs/cgroup cgroup2 rw,nosuid,nodev,noexec,relatime,nsdelegate 0 0
> > :~ # mount -t cgroup2 cgroup2 /sys/fs/cgroup/ -oremount
> > :~ # grep cg /proc/mounts
> > cgroup2 /sys/fs/cgroup cgroup2 rw,relatime 0 0
> 
> The mount(2) says about remounting:
> > The  mountflags  and  data  arguments should match the values used in
> > the original mount() call, except for those parameters that are being
> > deliberately changed.
>
> Or is this a provision for the fsconfig(2) API?

It's just me not knowing how these things work. I just looked at other real
filesystems and copied.

> > +	fsparam_flag("memory_nolocalevents",	Opt_memory_nolocalevents),
> > +	fsparam_flag("memory_norecursiveprot",	Opt_memory_norecursiveprot),
> 
> These are not 'no' prefixes of the option :-)

Oh, I tried that first but nomemory_recursiveprot looked really weird. The
thing is that the underbar is added to separate the subsystem from the
actual option and we're now prepending no to the subsystem part of the name.
I'm not super attached to the current names tho.

> I.e. it seem more consistent to prefix whole boolean option name (in
> accordance with other FS options but I know limited subset of them).
> In the end, this should be handled generically for boolean options in
> the VFS and not via custom options.
> 
> Also, this allows both
> 	'nsdelegate,nonsdelegate'
> and
> 	'nonsdelegate,nsdelegate'
> (nsdelegate is just an example) where the 'no' always overrides being a
> hidden implementation detail.
> 
> I find this patch a bit weird.

It is a bit weird. Lemme play a bit with turning off the options and I'll
remove the no options if they can be turned off without explicitly
specifying them.

Thanks.

-- 
tejun