Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pkg/system: Allow EnsureRemoveAll to delete trees with immutable files #1355

Merged
merged 1 commit into from
Sep 23, 2022

Conversation

dfr
Copy link
Contributor

@dfr dfr commented Sep 21, 2022

Fixes #1354.

Signed-off-by: Doug Rabson [email protected]

@rhatdan
Copy link
Member

rhatdan commented Sep 23, 2022

@nalind
Copy link
Member

nalind commented Sep 23, 2022

There's probably a corresponding function on Linux that uses the FS_IOC_SETFLAGS ioctl to clear immutable flags, but we don't tend to run into that in routine use, so we can leave that for another time.
LGTM.

@rhatdan rhatdan merged commit 57556f4 into containers:main Sep 23, 2022
@rhatdan
Copy link
Member

rhatdan commented Sep 23, 2022

int attr;
fd = open("pathname", ...);

ioctl(fd, FS_IOC_GETFLAGS, &attr);  /* Read current flags */
attr |= FS_NOATIME_FL;              /* Add desired flag */
ioctl(fd, FS_IOC_SETFLAGS, &attr);  /* Set updated flags */

The interesting part is the flags themselves. There are some that help optimize file-operations, others that manipulate the file operational mode and yes, there are always some exotic flags that even after reading their description twice I still don’t know how it would actually affect the system.

But, inside this haystack I found two shiny needles that really caught my eye and got my heart rate up, and yeah, I know this sounds nerdy :)

    FS_APPEND_FL: Mark a file, so that it can be opened only with the O_APPEND flag.
    FS_IMMUTABLE_FL: The file is immutable: no changes are permitted to the file contents or metadata (permissions, timestamps, ownership, link count and so on).

@rhatdan
Copy link
Member

rhatdan commented Sep 23, 2022

@rhatdan
Copy link
Member

rhatdan commented Sep 23, 2022

One issue with this is that it will be slow, since it needs to walk the trees twice.

Would it be better to RemoveAll then if it fails attempt to change the immutable and try to removeall again.

@dfr
Copy link
Contributor Author

dfr commented Sep 24, 2022

I think immutable and append flags are generally useful. BSDs have used immutable, combined with securelevel, to protect suid utilities like login and su for a very long time. The point about efficiency is a good one - I used WalkDir which should avoid excessive stat calls but still, it would be better to check for an EPERM error from RemoveAll first.

I was working on a separate PR which will preserve the BSD flags in pkg/archive but got slightly distracted by fixing tests on FreeBSD. I will add a commit to that to avoid the double tree walk in simple cases.

@dfr dfr deleted the freebsd-chflags branch September 24, 2022 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deleting images or containers with immutable files fails on FreeBSD
3 participants