On 22.04.2024 at 04:43, Karel Zak wrote: > On Thu, Apr 18, 2024 at 01:00:56AM -0700, Krzysztof Olędzki wrote: >> Clearly this is not the right fix, but perhaps something like this would be correct: >> >> @@ -275,6 +275,7 @@ >> || mnt_context_is_lazy(cxt) >> || mnt_context_is_nocanonicalize(cxt) >> || mnt_context_is_loopdel(cxt) >> + || mnt_context_is_rdonly_umount(cxt) >> || mnt_safe_stat(tgt, &st) != 0 || !S_ISDIR(st.st_mode) >> || has_utab_entry(cxt, tgt)) >> return 1; /* not found */ >> >> I wonder if we just missed the mnt_context_is_rdonly_umount case in https://git.kernel.org/pub/scm/utils/util-linux/util-linux.git/commit/?id=6a52473ecd877227f6f7da2b95da0b51593ffec1? > > Yes :-) > > Nice debugging work, thanks! Thanks! > I will most likely use the bugfix that you have suggested. The > long-term solution should be to use the new mount API for > context_umount.c as well, as it allows for reconfiguring the > mountpoint without having to worry about the source. > > attr.attr_set = MOUNT_ATTR_RDONLY; > > fd = open_tree(AT_FDCWD, target, 0); > mount_setattr(fd, "", AT_EMPTY_PATH, &attr, sizeof(attr)); > close(fd); > > However, we need to implement it in a backwardly compatible way for > cases where there is the new API unsupported. Right, and it seems it will need to live for many years to come given mount_setattr got added in Linux 5.12 with xfs/ext4/FAT support only and several other filesystems added in 5.15/5.18/5.19. BTW: we probably did not notice it for that long because Linux kernel has a special handling for root filesystem, where failed umount is internally handled as remount read-only, and it is not very common to have /usr on a separate filesystem. Thanks, Krzysztof
