On Tue, Nov 29, 2022 at 06:24:00AM -0800, Christoph Hellwig wrote:
> This seems to be missing any kind of changelog.  Also the
> subject should say file_operations.  Most of the instances here are
> not in a file system, and they most certainly aren't callbacks.
> 
> I think you've also missed a whole lot of maintainers.
> 
> > +#include "linux/security.h"
> 
> That's now how we include kernel-wide headers.
> 
> >  #include <linux/blkdev.h>
> >  #include <linux/blk-mq.h>
> >  #include <linux/blk-integrity.h>
> > @@ -3308,6 +3309,13 @@ static int nvme_dev_release(struct inode *inode, struct file *file)
> >  	return 0;
> >  }
> >  
> > +int nvme_uring_cmd_sec(struct io_uring_cmd *ioucmd,  struct security_uring_cmd *sec)
> 
> Douple white space and overly long line.
> 
> > +{
> > +	sec->flags = 0;
> > +	sec->flags = SECURITY_URING_CMD_TYPE_IOCTL;
> 
> Double initialization of ->flags.  But how is this supposed to work
> to start with?

This RFC is meant to see how different solutions may play out. I'm not
trying to push anything through yet. Just testing the waters to see what
sticks and what people think about certain approaches. Should have
mentioned that in my cover letter.

My idea was to bring all relevant maintainers into the conversation once
I had a more clear idea on what needed to be done and how I would do it.

Since the patch is just a discussion piece, it is riddled with errors
like the ones you pointed out.

The idea with this second version is to add a security uring callback to
the already existing ones in the file_operations structure. This new
callback will fill in a security struct that will contain all the data
needed for the LSMs to do their thing. This callback can be protected by
an 'ifdef' for performance purposes.

There is a third proposal by Ming Lei that uses the io_uring_sqe struct
to embed io_uring type information. In my todo list I have a task to
implement this and present it as a third option.

best
Joel