| Message ID | 20220125143304.34628-1-cgzones@googlemail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] mm: create security context for memfd_secret inodes | expand |
On Tue, Jan 25, 2022 at 9:33 AM Christian Göttsche <cgzones@googlemail.com> wrote: > > Create a security context for the inodes created by memfd_secret(2) via > the LSM hook inode_init_security_anon to allow a fine grained control. > As secret memory areas can affect hibernation and have a global shared > limit access control might be desirable. > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com> > --- > An alternative way of checking memfd_secret(2) is to create a new LSM > hook and e.g. for SELinux check via a new process class permission. > --- > mm/secretmem.c | 9 +++++++++ > 1 file changed, 9 insertions(+) This seems reasonable to me, and I like the idea of labeling the anon inode as opposed to creating a new set of LSM hooks. If we want to apply access control policy to the memfd_secret() fds we are going to need to attach some sort of LSM state to the inode, we might as well use the mechanism we already have instead of inventing another one. > diff --git a/mm/secretmem.c b/mm/secretmem.c > index 22b310adb53d..b61cd2f661bc 100644 > --- a/mm/secretmem.c > +++ b/mm/secretmem.c > @@ -164,11 +164,20 @@ static struct file *secretmem_file_create(unsigned long flags) > { > struct file *file = ERR_PTR(-ENOMEM); > struct inode *inode; > + const char *anon_name = "[secretmem]"; > + const struct qstr qname = QSTR_INIT(anon_name, strlen(anon_name)); > + int err; > > inode = alloc_anon_inode(secretmem_mnt->mnt_sb); > if (IS_ERR(inode)) > return ERR_CAST(inode); > > + err = security_inode_init_security_anon(inode, &qname, NULL); > + if (err) { > + file = ERR_PTR(err); > + goto err_free_inode; > + } > + > file = alloc_file_pseudo(inode, secretmem_mnt, "secretmem", > O_RDWR, &secretmem_fops); > if (IS_ERR(file)) > -- > 2.34.1
On Thu, 27 Jan 2022 at 00:01, Paul Moore <paul@paul-moore.com> wrote: > > On Tue, Jan 25, 2022 at 9:33 AM Christian Göttsche > <cgzones@googlemail.com> wrote: > > > > Create a security context for the inodes created by memfd_secret(2) via > > the LSM hook inode_init_security_anon to allow a fine grained control. > > As secret memory areas can affect hibernation and have a global shared > > limit access control might be desirable. > > > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com> > > --- > > An alternative way of checking memfd_secret(2) is to create a new LSM > > hook and e.g. for SELinux check via a new process class permission. > > --- > > mm/secretmem.c | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > This seems reasonable to me, and I like the idea of labeling the anon > inode as opposed to creating a new set of LSM hooks. If we want to > apply access control policy to the memfd_secret() fds we are going to > need to attach some sort of LSM state to the inode, we might as well > use the mechanism we already have instead of inventing another one. Any further comments (on design or implementation)? Should I resend a non-rfc? One naming question: Should the anonymous inode class be named "[secretmem]", like "[userfaultfd]", or "[secret_mem]" similar to "[io_uring]"? > > diff --git a/mm/secretmem.c b/mm/secretmem.c > > index 22b310adb53d..b61cd2f661bc 100644 > > --- a/mm/secretmem.c > > +++ b/mm/secretmem.c > > @@ -164,11 +164,20 @@ static struct file *secretmem_file_create(unsigned long flags) > > { > > struct file *file = ERR_PTR(-ENOMEM); > > struct inode *inode; > > + const char *anon_name = "[secretmem]"; > > + const struct qstr qname = QSTR_INIT(anon_name, strlen(anon_name)); > > + int err; > > > > inode = alloc_anon_inode(secretmem_mnt->mnt_sb); > > if (IS_ERR(inode)) > > return ERR_CAST(inode); > > > > + err = security_inode_init_security_anon(inode, &qname, NULL); > > + if (err) { > > + file = ERR_PTR(err); > > + goto err_free_inode; > > + } > > + > > file = alloc_file_pseudo(inode, secretmem_mnt, "secretmem", > > O_RDWR, &secretmem_fops); > > if (IS_ERR(file)) > > -- > > 2.34.1 > > -- > paul-moore.com
On Thu, Feb 17, 2022 at 9:24 AM Christian Göttsche <cgzones@googlemail.com> wrote: > On Thu, 27 Jan 2022 at 00:01, Paul Moore <paul@paul-moore.com> wrote: > > On Tue, Jan 25, 2022 at 9:33 AM Christian Göttsche > > <cgzones@googlemail.com> wrote: > > > > > > Create a security context for the inodes created by memfd_secret(2) via > > > the LSM hook inode_init_security_anon to allow a fine grained control. > > > As secret memory areas can affect hibernation and have a global shared > > > limit access control might be desirable. > > > > > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com> > > > --- > > > An alternative way of checking memfd_secret(2) is to create a new LSM > > > hook and e.g. for SELinux check via a new process class permission. > > > --- > > > mm/secretmem.c | 9 +++++++++ > > > 1 file changed, 9 insertions(+) > > > > This seems reasonable to me, and I like the idea of labeling the anon > > inode as opposed to creating a new set of LSM hooks. If we want to > > apply access control policy to the memfd_secret() fds we are going to > > need to attach some sort of LSM state to the inode, we might as well > > use the mechanism we already have instead of inventing another one. > > Any further comments (on design or implementation)? > > Should I resend a non-rfc? I personally would really like to see a selinux-testsuite for this so that we can verify it works not just now but in the future too. I think having a test would also help demonstrate the usefulness of the additional LSM controls. > One naming question: > Should the anonymous inode class be named "[secretmem]", like > "[userfaultfd]", or "[secret_mem]" similar to "[io_uring]"? The pr_fmt() string in mm/secretmem.c uses "secretmem" so I would suggest sticking with "[secretmem]", although that is question best answered by the secretmem maintainer.
On Thu, 17 Feb 2022 at 23:32, Paul Moore <paul@paul-moore.com> wrote: > > On Thu, Feb 17, 2022 at 9:24 AM Christian Göttsche > <cgzones@googlemail.com> wrote: > > On Thu, 27 Jan 2022 at 00:01, Paul Moore <paul@paul-moore.com> wrote: > > > On Tue, Jan 25, 2022 at 9:33 AM Christian Göttsche > > > <cgzones@googlemail.com> wrote: > > > > > > > > Create a security context for the inodes created by memfd_secret(2) via > > > > the LSM hook inode_init_security_anon to allow a fine grained control. > > > > As secret memory areas can affect hibernation and have a global shared > > > > limit access control might be desirable. > > > > > > > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com> > > > > --- > > > > An alternative way of checking memfd_secret(2) is to create a new LSM > > > > hook and e.g. for SELinux check via a new process class permission. > > > > --- > > > > mm/secretmem.c | 9 +++++++++ > > > > 1 file changed, 9 insertions(+) > > > > > > This seems reasonable to me, and I like the idea of labeling the anon > > > inode as opposed to creating a new set of LSM hooks. If we want to > > > apply access control policy to the memfd_secret() fds we are going to > > > need to attach some sort of LSM state to the inode, we might as well > > > use the mechanism we already have instead of inventing another one. > > > > Any further comments (on design or implementation)? > > > > Should I resend a non-rfc? > > I personally would really like to see a selinux-testsuite for this so > that we can verify it works not just now but in the future too. I > think having a test would also help demonstrate the usefulness of the > additional LSM controls. > Any comments (especially from the mm people)? Draft SELinux testsuite patch: https://github.com/SELinuxProject/selinux-testsuite/pull/80 > > One naming question: > > Should the anonymous inode class be named "[secretmem]", like > > "[userfaultfd]", or "[secret_mem]" similar to "[io_uring]"? > > The pr_fmt() string in mm/secretmem.c uses "secretmem" so I would > suggest sticking with "[secretmem]", although that is question best > answered by the secretmem maintainer. > > -- > paul-moore.com
On Mon, May 2, 2022 at 9:45 AM Christian Göttsche <cgzones@googlemail.com> wrote: > On Thu, 17 Feb 2022 at 23:32, Paul Moore <paul@paul-moore.com> wrote: > > On Thu, Feb 17, 2022 at 9:24 AM Christian Göttsche > > <cgzones@googlemail.com> wrote: > > > On Thu, 27 Jan 2022 at 00:01, Paul Moore <paul@paul-moore.com> wrote: > > > > On Tue, Jan 25, 2022 at 9:33 AM Christian Göttsche > > > > <cgzones@googlemail.com> wrote: > > > > > > > > > > Create a security context for the inodes created by memfd_secret(2) via > > > > > the LSM hook inode_init_security_anon to allow a fine grained control. > > > > > As secret memory areas can affect hibernation and have a global shared > > > > > limit access control might be desirable. > > > > > > > > > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com> > > > > > --- > > > > > An alternative way of checking memfd_secret(2) is to create a new LSM > > > > > hook and e.g. for SELinux check via a new process class permission. > > > > > --- > > > > > mm/secretmem.c | 9 +++++++++ > > > > > 1 file changed, 9 insertions(+) > > > > > > > > This seems reasonable to me, and I like the idea of labeling the anon > > > > inode as opposed to creating a new set of LSM hooks. If we want to > > > > apply access control policy to the memfd_secret() fds we are going to > > > > need to attach some sort of LSM state to the inode, we might as well > > > > use the mechanism we already have instead of inventing another one. > > > > > > Any further comments (on design or implementation)? > > > > > > Should I resend a non-rfc? > > > > I personally would really like to see a selinux-testsuite for this so > > that we can verify it works not just now but in the future too. I > > think having a test would also help demonstrate the usefulness of the > > additional LSM controls. > > > > Any comments (especially from the mm people)? > > Draft SELinux testsuite patch: > https://github.com/SELinuxProject/selinux-testsuite/pull/80 > > > > One naming question: > > > Should the anonymous inode class be named "[secretmem]", like > > > "[userfaultfd]", or "[secret_mem]" similar to "[io_uring]"? > > > > The pr_fmt() string in mm/secretmem.c uses "secretmem" so I would > > suggest sticking with "[secretmem]", although that is question best > > answered by the secretmem maintainer. I think this patchset has been posted for long enough with no comments, and no objections, that I can pull this into the selinux/next tree. However, I'll give it until the end of this week just to give folks one last chance to comment. If I don't hear any objections by the end of day on Friday, June 10th I'll go ahead and merge this.
On Tue, Jun 7, 2022 at 4:10 PM Paul Moore <paul@paul-moore.com> wrote: > On Mon, May 2, 2022 at 9:45 AM Christian Göttsche > <cgzones@googlemail.com> wrote: > > On Thu, 17 Feb 2022 at 23:32, Paul Moore <paul@paul-moore.com> wrote: > > > On Thu, Feb 17, 2022 at 9:24 AM Christian Göttsche > > > <cgzones@googlemail.com> wrote: > > > > On Thu, 27 Jan 2022 at 00:01, Paul Moore <paul@paul-moore.com> wrote: > > > > > On Tue, Jan 25, 2022 at 9:33 AM Christian Göttsche > > > > > <cgzones@googlemail.com> wrote: > > > > > > > > > > > > Create a security context for the inodes created by memfd_secret(2) via > > > > > > the LSM hook inode_init_security_anon to allow a fine grained control. > > > > > > As secret memory areas can affect hibernation and have a global shared > > > > > > limit access control might be desirable. > > > > > > > > > > > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com> > > > > > > --- > > > > > > An alternative way of checking memfd_secret(2) is to create a new LSM > > > > > > hook and e.g. for SELinux check via a new process class permission. > > > > > > --- > > > > > > mm/secretmem.c | 9 +++++++++ > > > > > > 1 file changed, 9 insertions(+) > > > > > > > > > > This seems reasonable to me, and I like the idea of labeling the anon > > > > > inode as opposed to creating a new set of LSM hooks. If we want to > > > > > apply access control policy to the memfd_secret() fds we are going to > > > > > need to attach some sort of LSM state to the inode, we might as well > > > > > use the mechanism we already have instead of inventing another one. > > > > > > > > Any further comments (on design or implementation)? > > > > > > > > Should I resend a non-rfc? > > > > > > I personally would really like to see a selinux-testsuite for this so > > > that we can verify it works not just now but in the future too. I > > > think having a test would also help demonstrate the usefulness of the > > > additional LSM controls. > > > > > > > Any comments (especially from the mm people)? > > > > Draft SELinux testsuite patch: > > https://github.com/SELinuxProject/selinux-testsuite/pull/80 > > > > > > One naming question: > > > > Should the anonymous inode class be named "[secretmem]", like > > > > "[userfaultfd]", or "[secret_mem]" similar to "[io_uring]"? > > > > > > The pr_fmt() string in mm/secretmem.c uses "secretmem" so I would > > > suggest sticking with "[secretmem]", although that is question best > > > answered by the secretmem maintainer. > > I think this patchset has been posted for long enough with no > comments, and no objections, that I can pull this into the > selinux/next tree. However, I'll give it until the end of this week > just to give folks one last chance to comment. If I don't hear any > objections by the end of day on Friday, June 10th I'll go ahead and > merge this. I didn't see any comments so I just merged this into selinux/next. Thanks for your patience Christian.
diff --git a/mm/secretmem.c b/mm/secretmem.c index 22b310adb53d..b61cd2f661bc 100644 --- a/mm/secretmem.c +++ b/mm/secretmem.c @@ -164,11 +164,20 @@ static struct file *secretmem_file_create(unsigned long flags) { struct file *file = ERR_PTR(-ENOMEM); struct inode *inode; + const char *anon_name = "[secretmem]"; + const struct qstr qname = QSTR_INIT(anon_name, strlen(anon_name)); + int err; inode = alloc_anon_inode(secretmem_mnt->mnt_sb); if (IS_ERR(inode)) return ERR_CAST(inode); + err = security_inode_init_security_anon(inode, &qname, NULL); + if (err) { + file = ERR_PTR(err); + goto err_free_inode; + } + file = alloc_file_pseudo(inode, secretmem_mnt, "secretmem", O_RDWR, &secretmem_fops); if (IS_ERR(file))
Create a security context for the inodes created by memfd_secret(2) via the LSM hook inode_init_security_anon to allow a fine grained control. As secret memory areas can affect hibernation and have a global shared limit access control might be desirable. Signed-off-by: Christian Göttsche <cgzones@googlemail.com> --- An alternative way of checking memfd_secret(2) is to create a new LSM hook and e.g. for SELinux check via a new process class permission. --- mm/secretmem.c | 9 +++++++++ 1 file changed, 9 insertions(+)