[PATCH 0/2] Create io_uring fd with ephemeral inode

[PATCH 0/2] Create io_uring fd with ephemeral inode

[Kumar Kartikeya Dwivedi]

This set converts io_uring to use secure anon_inodes (with a newly allocated
non-S_PRIVATE inode) for each individual instance. In addition to allowing LSM
modules to enforce policy using the inode context, it also enables
checkpoint/restore usecases by allowing mapping the VMA to the open fd in a
task. Offset is already available to determine rings mapped per region, so this
was the only missing piece in establishing region <-> io_uring instance mapping.

LSM tie up has been left out of this set for now.

Kumar Kartikeya Dwivedi (2):
  fs: anon_inodes: export anon_inode_getfile_secure helper
  fs: io_uring: convert to use anon_inode_getfile_secure

 fs/anon_inodes.c            | 9 +++++++++
 fs/io_uring.c               | 4 ++--
 include/linux/anon_inodes.h | 4 ++++
 3 files changed, 15 insertions(+), 2 deletions(-)

-- 
2.31.1

[PATCH 1/2] fs: anon_inodes: export anon_inode_getfile_secure helper

[Kumar Kartikeya Dwivedi]

This is the non-fd installing analogue of anon_inode_getfd_secure. In
addition to allowing LSMs to attach policy to the distinct inode, this
is also needed for checkpoint restore of an io_uring instance where a
mapped region needs to mapped back to the io_uring fd by CRIU. This is
currently not possible as all anon_inodes share a single inode.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 fs/anon_inodes.c            | 9 +++++++++
 include/linux/anon_inodes.h | 4 ++++
 2 files changed, 13 insertions(+)

diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
index a280156138ed..37032786b211 100644
--- a/fs/anon_inodes.c
+++ b/fs/anon_inodes.c
@@ -148,6 +148,15 @@ struct file *anon_inode_getfile(const char *name,
 }
 EXPORT_SYMBOL_GPL(anon_inode_getfile);
 
+struct file *anon_inode_getfile_secure(const char *name,
+				       const struct file_operations *fops,
+				       void *priv, int flags,
+				       const struct inode *context_inode)
+{
+	return __anon_inode_getfile(name, fops, priv, flags, context_inode, true);
+}
+EXPORT_SYMBOL_GPL(anon_inode_getfile_secure);
+
 static int __anon_inode_getfd(const char *name,
 			      const struct file_operations *fops,
 			      void *priv, int flags,
diff --git a/include/linux/anon_inodes.h b/include/linux/anon_inodes.h
index 71881a2b6f78..5deaddbd7927 100644
--- a/include/linux/anon_inodes.h
+++ b/include/linux/anon_inodes.h
@@ -15,6 +15,10 @@ struct inode;
 struct file *anon_inode_getfile(const char *name,
 				const struct file_operations *fops,
 				void *priv, int flags);
+struct file *anon_inode_getfile_secure(const char *name,
+				       const struct file_operations *fops,
+				       void *priv, int flags,
+				       const struct inode *context_inode);
 int anon_inode_getfd(const char *name, const struct file_operations *fops,
 		     void *priv, int flags);
 int anon_inode_getfd_secure(const char *name,
-- 
2.31.1

Re: [PATCH 1/2] fs: anon_inodes: export anon_inode_getfile_secure helper

[Paul Moore]

On Wed, May 19, 2021 at 7:37 AM Kumar Kartikeya Dwivedi
<memxor@gmail.com> wrote:
>
> This is the non-fd installing analogue of anon_inode_getfd_secure. In
> addition to allowing LSMs to attach policy to the distinct inode, this
> is also needed for checkpoint restore of an io_uring instance where a
> mapped region needs to mapped back to the io_uring fd by CRIU. This is
> currently not possible as all anon_inodes share a single inode.
>
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> ---
>  fs/anon_inodes.c            | 9 +++++++++
>  include/linux/anon_inodes.h | 4 ++++
>  2 files changed, 13 insertions(+)

[NOTE: dropping dancol@google as that email is bouncy]

> diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
> index a280156138ed..37032786b211 100644
> --- a/fs/anon_inodes.c
> +++ b/fs/anon_inodes.c
> @@ -148,6 +148,15 @@ struct file *anon_inode_getfile(const char *name,
>  }
>  EXPORT_SYMBOL_GPL(anon_inode_getfile);

This function should have a comment block at the top similar to
anon_inode_getfile(); in fact you can likely copy-n-paste the bulk of
it to use as a start.

If you don't want to bother respinning, I've got this exact patch
(+comments) in my patchset that I'll post later and I'm happy to
give/share credit if that is important to you.

> +struct file *anon_inode_getfile_secure(const char *name,
> +                                      const struct file_operations *fops,
> +                                      void *priv, int flags,
> +                                      const struct inode *context_inode)
> +{
> +       return __anon_inode_getfile(name, fops, priv, flags, context_inode, true);
> +}
> +EXPORT_SYMBOL_GPL(anon_inode_getfile_secure);

-- 
paul moore
www.paul-moore.com

Re: [PATCH 1/2] fs: anon_inodes: export anon_inode_getfile_secure helper

[Kumar Kartikeya Dwivedi]

On Wed, May 19, 2021 at 08:52:51PM IST, Paul Moore wrote:
> On Wed, May 19, 2021 at 7:37 AM Kumar Kartikeya Dwivedi
> <memxor@gmail.com> wrote:
> >
> > This is the non-fd installing analogue of anon_inode_getfd_secure. In
> > addition to allowing LSMs to attach policy to the distinct inode, this
> > is also needed for checkpoint restore of an io_uring instance where a
> > mapped region needs to mapped back to the io_uring fd by CRIU. This is
> > currently not possible as all anon_inodes share a single inode.
> >
> > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> > ---
> >  fs/anon_inodes.c            | 9 +++++++++
> >  include/linux/anon_inodes.h | 4 ++++
> >  2 files changed, 13 insertions(+)
>
> [NOTE: dropping dancol@google as that email is bouncy]
>
> > diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
> > index a280156138ed..37032786b211 100644
> > --- a/fs/anon_inodes.c
> > +++ b/fs/anon_inodes.c
> > @@ -148,6 +148,15 @@ struct file *anon_inode_getfile(const char *name,
> >  }
> >  EXPORT_SYMBOL_GPL(anon_inode_getfile);
>
> This function should have a comment block at the top similar to
> anon_inode_getfile(); in fact you can likely copy-n-paste the bulk of
> it to use as a start.
>
> If you don't want to bother respinning, I've got this exact patch
> (+comments) in my patchset that I'll post later and I'm happy to
> give/share credit if that is important to you.
>

That'd be great; no credit is fine :). Please CC me when you post it.

> > +struct file *anon_inode_getfile_secure(const char *name,
> > +                                      const struct file_operations *fops,
> > +                                      void *priv, int flags,
> > +                                      const struct inode *context_inode)
> > +{
> > +       return __anon_inode_getfile(name, fops, priv, flags, context_inode, true);
> > +}
> > +EXPORT_SYMBOL_GPL(anon_inode_getfile_secure);
>
> --
> paul moore
> www.paul-moore.com

--
Kartikeya

Re: [PATCH 1/2] fs: anon_inodes: export anon_inode_getfile_secure helper

[Paul Moore]

On Wed, May 19, 2021 at 7:07 PM Kumar Kartikeya Dwivedi
<memxor@gmail.com> wrote:
> On Wed, May 19, 2021 at 08:52:51PM IST, Paul Moore wrote:
> > On Wed, May 19, 2021 at 7:37 AM Kumar Kartikeya Dwivedi
> > <memxor@gmail.com> wrote:
> > >
> > > This is the non-fd installing analogue of anon_inode_getfd_secure. In
> > > addition to allowing LSMs to attach policy to the distinct inode, this
> > > is also needed for checkpoint restore of an io_uring instance where a
> > > mapped region needs to mapped back to the io_uring fd by CRIU. This is
> > > currently not possible as all anon_inodes share a single inode.
> > >
> > > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> > > ---
> > >  fs/anon_inodes.c            | 9 +++++++++
> > >  include/linux/anon_inodes.h | 4 ++++
> > >  2 files changed, 13 insertions(+)
> >
> > [NOTE: dropping dancol@google as that email is bouncy]
> >
> > > diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
> > > index a280156138ed..37032786b211 100644
> > > --- a/fs/anon_inodes.c
> > > +++ b/fs/anon_inodes.c
> > > @@ -148,6 +148,15 @@ struct file *anon_inode_getfile(const char *name,
> > >  }
> > >  EXPORT_SYMBOL_GPL(anon_inode_getfile);
> >
> > This function should have a comment block at the top similar to
> > anon_inode_getfile(); in fact you can likely copy-n-paste the bulk of
> > it to use as a start.
> >
> > If you don't want to bother respinning, I've got this exact patch
> > (+comments) in my patchset that I'll post later and I'm happy to
> > give/share credit if that is important to you.
> >
>
> That'd be great; no credit is fine :). Please CC me when you post it.

Will do.  I dug out my system which had the patches and I'm working on
forward porting them to v5.13-rc2; if I don't have them cleaned up
enough for posting by Thursday, I'll make sure they are at least RFC
ready by Friday.

-- 
paul moore
www.paul-moore.com

Re: [PATCH 1/2] fs: anon_inodes: export anon_inode_getfile_secure helper

[Christoph Hellwig]

On Wed, May 19, 2021 at 05:00:56PM +0530, Kumar Kartikeya Dwivedi wrote:
> This is the non-fd installing analogue of anon_inode_getfd_secure. In
> addition to allowing LSMs to attach policy to the distinct inode, this
> is also needed for checkpoint restore of an io_uring instance where a
> mapped region needs to mapped back to the io_uring fd by CRIU. This is
> currently not possible as all anon_inodes share a single inode.

No need to export it, as io_uring can't be built modular.

> +struct file *anon_inode_getfile_secure(const char *name,
> +				       const struct file_operations *fops,
> +				       void *priv, int flags,
> +				       const struct inode *context_inode)
> +{
> +	return __anon_inode_getfile(name, fops, priv, flags, context_inode, true);
> +}
> +EXPORT_SYMBOL_GPL(anon_inode_getfile_secure);

Please avoid the overly long line here.

[PATCH 2/2] fs: io_uring: convert to use anon_inode_getfile_secure

[Kumar Kartikeya Dwivedi]

Make io_uring use anon_inode_getfile_secure helper exposed in previous
commit. This gives each io_uring instance a distinct inode.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 fs/io_uring.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 65a17d560a73..001fcfb63f33 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -9454,8 +9454,8 @@ static struct file *io_uring_get_file(struct io_ring_ctx *ctx)
 		return ERR_PTR(ret);
 #endif
 
-	file = anon_inode_getfile("[io_uring]", &io_uring_fops, ctx,
-					O_RDWR | O_CLOEXEC);
+	file = anon_inode_getfile_secure("[io_uring]", &io_uring_fops, ctx,
+					 O_RDWR | O_CLOEXEC, NULL);
 #if defined(CONFIG_UNIX)
 	if (IS_ERR(file)) {
 		sock_release(ctx->ring_sock);
-- 
2.31.1

Re: [PATCH 0/2] Create io_uring fd with ephemeral inode

[Paul Moore]

On Wed, May 19, 2021 at 7:37 AM Kumar Kartikeya Dwivedi
<memxor@gmail.com> wrote:
>
> This set converts io_uring to use secure anon_inodes (with a newly allocated
> non-S_PRIVATE inode) for each individual instance. In addition to allowing LSM
> modules to enforce policy using the inode context, it also enables
> checkpoint/restore usecases by allowing mapping the VMA to the open fd in a
> task. Offset is already available to determine rings mapped per region, so this
> was the only missing piece in establishing region <-> io_uring instance mapping.
>
> LSM tie up has been left out of this set for now.

This brings to light something I have been trying to resolve for a
little while now, but I have been finding it difficult to find the
necessary time due to competing priorities at work and in my personal
time.  While the patches in this patchset are a necessary dependency,
there are other issues which remain unresolved but which are now
public (although the problems were not buried very far in the first
place).  Further complicating things on my end is that the system with
my current work-in-progress patchset was taken offline two days ago
and my office is under renovations :/

Give me a day or two to get the patches off that system and I'll post
them here and we can start the process of kicking around solutions
that work for everyone.

-- 
paul moore
www.paul-moore.com