GNOME Bugzilla – Bug 728006
optionally make use of S_IMMUTABLE
Last modified: 2014-06-09 18:44:33 UTC
See sealing discussion: http://lwn.net/Articles/593918/ Now I dinged Nix for using S_IMMUTABLE, but maybe it is better to go belt-and-suspenders on this. The downside is that we have to remove the bit temporarily when making a hardlink, but that's not difficult to do. It still would be nice if the kernel had a S_IMMUTABLE_CONTENT or so.
See also http://marc.info/?l=linux-fsdevel&m=139963046823575&w=2
Created attachment 277562 [details] [review] deploy: Set the immutable bit on the deployment root This prevents people from creating new directories there and expecting them to be persisted. The OSTree model has all local state to be in /etc and /var. This introduces a compile-time dependency on libe2fsprogs.
This is just an initial step; a further step we could take now would be to make subdirectories immutable (/usr, /usr/etc) in particular would be good targets.
Created attachment 277571 [details] [review] deploy: Set the immutable bit on the deployment root We need to check for ENOTTY for ioctl(), not EOPNOTSUPP. However I left in the EOPNOTSUPP just in case.
Review of attachment 277571 [details] [review]: Overall, code looks fine, no obvious defects I can find. Patch applies and builds cleanly. It would be worth a double-check to make sure that all common root filesystems support immutable - I know the EXT family does, but it's possible there's a strange one (one of the SSD/Flash optimized FSes, perhaps?) that does not. The change of root to immutable is definitely sensible in an OSTree managed system. I was initially convinced it was a bad idea to enable by default, but I can't think of any logical reasons for that. The only real reason I can think of to create a directory in root on an OSTree system would be to make a temporary folder, maybe to mount a volume, and best practice would be to do those elsewhere regardless.
Thanks for the review! I did check the kernel source to make sure it was supported by the important filesystems; "git grep FS_IOC_GETFLAGS" Attachment 277571 [details] pushed as b4d21e9 - deploy: Set the immutable bit on the deployment root
This regressed /var being remounted writable. See: https://git.gnome.org/browse/ostree/tree/src/switchroot/ostree-remount.c#n46 We need a more intelligent check here. Parsing the mount options?
Created attachment 277834 [details] [review] ostree-remount: Check for / being *mounted* read-only, not necessarily writable The previous S_IMMUTABLE commit broke ostree-remount; / is now not actually writable. All we really wanted to know though was whether it was *mounted* writable, so check that via statvfs() which is cleaner anyways (i.e. not via access() which kernel people hate).
Review of attachment 277834 [details] [review]: Looks good.
Attachment 277834 [details] pushed as cb43d29 - ostree-remount: Check for / being *mounted* read-only, not necessarily writable
Another regression: When /tmp is a regular filesystem, and we're running ostree as non-root (as the installed tests do), we want to ignore EPERM. I didn't notice this on my laptop because it's RHEL7 Workstation default of tmpfs-on-/tmp, but the current gnome-continuous build server is RHEL6, and I'm seeing: http://build.gnome.org/continuous/buildmaster/builds/2014/06/04/38/integrationtest/work-gnome-continuous-x86_64-runtime/installed-test-results/ostree_test-admin-deploy-switch.test/output.txt
Created attachment 277910 [details] [review] libostree: Silently ignore EPERM when setting EXT2_IMMUTABLE_FL In the case of running ostree as non-root on a regular filesystem (not tmpfs which doesn't support immutable), we should just silently do nothing if we encounter EPERM. Cache the result to avoid spam in strace.
Review of attachment 277910 [details] [review]: OK.
Attachment 277910 [details] pushed as e31daf4 - libostree: Silently ignore EPERM when setting EXT2_IMMUTABLE_FL