Re: [PATCH] security/landlock: use square brackets around "landlock-ruleset"
From: Mickaël Salaün <mic@digikod.net>
Date: 2021-10-15 11:55:19
Also in:
linux-security-module, selinux, stable
CCing linux-api and stable to give them a chance to confirm that changing proc symlink content is OK. On 15/10/2021 11:10, Christian Brauner wrote:
On Wed, Oct 13, 2021 at 05:47:53PM +0200, Mickaël Salaün wrote:quoted
On 12/10/2021 23:09, Paul Moore wrote:quoted
On Tue, Oct 12, 2021 at 4:38 PM Ondrej Mosnacek [off-list ref] wrote:quoted
On Tue, Oct 12, 2021 at 8:12 PM Paul Moore [off-list ref] wrote:quoted
On Tue, Oct 12, 2021 at 6:38 AM Christian Brauner [off-list ref] wrote:quoted
On Mon, Oct 11, 2021 at 04:38:55PM +0200, Mickaël Salaün wrote:quoted
On 11/10/2021 15:37, Christian Brauner wrote:quoted
From: Christian Brauner <redacted> Make the name of the anon inode fd "[landlock-ruleset]" instead of "landlock-ruleset". This is minor but most anon inode fds already carry square brackets around their name: [eventfd] [eventpoll] [fanotify] [fscontext] [io_uring] [pidfd] [signalfd] [timerfd] [userfaultfd] For the sake of consistency lets do the same for the landlock-ruleset anon inode fd that comes with landlock. We did the same in 1cdc415f1083 ("uapi, fsopen: use square brackets around "fscontext" [ver #2]") for the new mount api.Before creating "landlock-ruleset" FD, I looked at other anonymous FD and saw this kind of inconsistency. I don't get why we need to add extra characters to names, those brackets seem useless. If it should be partPast inconsistency shouldn't justify future inconsistency. If you have a strong opinion about this for landlock I'm not going to push for it. Exchanging more than 2-3 email about something like this seems too much.[NOTE: adding the SELinux list as well as Chris (SELinux refrence policy maintainer) and Petr (Fedora/RHEL SELinux)] Chris and Petr, do either of you currently have any policy that references the "landlock-ruleset" anonymous inode? In other words, would adding the brackets around the name cause you any problems?AFAIU, the anon_inode transitions (the only mechanism where the "file name" would be exposed to the policy) are done only for inodes created by anon_inode_getfd_secure(), which is currently only used by userfaultfd. So you don't even need to ask that question; at this point it should be safe to change any of the names except "[userfaultfd]" as far as SELinux policy is concerned.There is also io_uring if you look at selinux/next. Regardless, thanks, I didn't check to see if landlock was using the new anon inode interface, since both Mickaël and Christian were concerned about breaking SELinux I had assumed they were using it :)Ok, thanks Paul and Ondrej. Such anonymous inode names seem to be only exposed to proc for now. Let's change this name then. I think it make sense to backport this patch down to 5.13 to fix all the inconsistencies.Thank you. I do appreciate the point about this being annoying that we have this inconsistency and it has bothered me too. Christian