Re: [PATCH 1/1] mount_util: fix mount_flags entries for MS_RDONLY

[Bernd Schubert <bernd@bsbernd.com>]

On 5/8/26 17:44, Darrick J. Wong wrote:
> On Fri, May 08, 2026 at 12:43:14PM +0200, Bernd Schubert wrote:
>>
>>
>> On 5/8/26 00:13, Darrick J. Wong wrote:
>>> From: Darrick J. Wong <djwong@kernel.org>
>>>
>>> MS_RDONLY maps to MOUNT_ATTR_RDONLY in the new fsmount API, but the
>>> table omitted that.  Fix that.
>>>
>>> Fixes: 0d7e72541564a5 ("Unify mount flag structures and remove redundant is_mount_attr field")
>>> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
>>> ---
>>>  lib/mount_util.c |    4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>>
>>> diff --git a/lib/mount_util.c b/lib/mount_util.c
>>> index 1a0aec9bfe1a70..7ab1dba91e28e3 100644
>>> --- a/lib/mount_util.c
>>> +++ b/lib/mount_util.c
>>> @@ -110,8 +110,8 @@
>>>  
>>>  const struct mount_flags mount_flags[] = {
>>>  /* opt            flag             on  safe  mount_attr */
>>> -{"rw",           MS_RDONLY,        0,  1,    0},                      /* fsconfig */
>>> -{"ro",           MS_RDONLY,        1,  1,    0},                      /* fsconfig */
>>> +{"rw",           MS_RDONLY,        0,  1,    MOUNT_ATTR_RDONLY},     /* fsconfig */
>>> +{"ro",           MS_RDONLY,        1,  1,    MOUNT_ATTR_RDONLY},     /* fsconfig */
>>>  {"suid",         MS_NOSUID,        0,  0,    MOUNT_ATTR_NOSUID},     /* fsmount  */
>>>  {"nosuid",       MS_NOSUID,        1,  1,    MOUNT_ATTR_NOSUID},     /* fsmount  */
>>>  {"dev",          MS_NODEV,         0,  1,    MOUNT_ATTR_NODEV},      /* fsmount  */
>>>
>>
>> Hi Darrick,
>>
>> I had already pulled this the night before and it fails the new tests as
>> well. I think issue is that ro needs to be done via fsmount(...,
>> MOUNT_ATTR_RDONLY) *and* fsconfig(SET_FLAG, "ro"). I'm going to add a
>> new column into const struct mount_flags mount_flags[].
> 
> Ewww, you're right.  util-linux also special-cases this, and in a weird
> way:
> 
>        Since version 2.41, libmount has the ability to use optional
>        arguments vfs and fs (e.g. ro=fs) to specify where the read-only
>        setting should be applied. For example, using the command:
> 
>           mount -o ro=vfs /dev/sdc1 /A
> 
>        will mount the filesystem as read-write on the superblock level,
>        but the /A node will be set as read-only. In previous versions,
>        this required an additional "-o bind,remount,ro" operation to
>        achieve the same result.
> 
> "ro=vfs" means apply it to the mount but don't tell the filesystem;
> "ro=fs" means apply tell it to the filesystem, but not the mount;
> and plain "ro" means apply it to both?  Apparently??
> 

I just sent out a new patch series that fixes plain "ro". Thanks for
looking it up in util-linux, I see "ro=vfs/fs" as new feature and
already spent the entire (public holi)day on this series (there was
another critical sync-init issue, when sync-init was actually not used).
I.e. I'm going to ignore the "weird ways" for now.


Thanks,
Bernd