From: Christian Brauner <brauner@kernel.org> Date: 2021-10-11 13:37:16
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.
Cc: Mickaël Salaün <mic@digikod.net>
Cc: linux-security-module@vger.kernel.org
Signed-off-by: Christian Brauner <redacted>
---
security/landlock/syscalls.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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 part
of the interface, why is it not enforced by anon_inode_getfd()?
There is a lot of other names that come without brackets (e.g. inotify,
bpf-*, btf, kvm-*, iio*). Do you plan to send patches for those too?
Changing such FD names could break user space because they may already
be exposed and used (e.g. through SELinux).
From: Christian Brauner <hidden> Date: 2021-10-12 10:38:35
On Mon, Oct 11, 2021 at 04:38:55PM +0200, Mickaël Salaün wrote:
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 part
Past 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.
of the interface, why is it not enforced by anon_inode_getfd()?
Sure, we can add that too.
There is a lot of other names that come without brackets (e.g. inotify,
bpf-*, btf, kvm-*, iio*). Do you plan to send patches for those too?
Changing such FD names could break user space because they may already
be exposed and used (e.g. through SELinux).
We didn't do it for bpf and kvm stuff because it has been that way for
a long time. We try to do it for all new ones.
Christian
From: Paul Moore <paul@paul-moore.com> Date: 2021-10-12 18:12:19
On Tue, Oct 12, 2021 at 6:38 AM Christian Brauner
[off-list ref] wrote:
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 part
Past 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?
--
paul moore
www.paul-moore.com
From: Ondrej Mosnacek <omosnace@redhat.com> Date: 2021-10-12 20:38:51
On Tue, Oct 12, 2021 at 8:12 PM Paul Moore [off-list ref] wrote:
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 part
Past 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.
--
Ondrej Mosnacek
Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.
From: Paul Moore <paul@paul-moore.com> Date: 2021-10-12 21:10:06
On Tue, Oct 12, 2021 at 4:38 PM Ondrej Mosnacek [off-list ref] wrote:
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 part
Past 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 :)
--
paul moore
www.paul-moore.com
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 part
Past 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.
From: Christian Brauner <hidden> Date: 2021-10-15 09:10:18
On Wed, Oct 13, 2021 at 05:47:53PM +0200, Mickaël Salaün wrote:
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 part
Past 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
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 part
Past 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