On Wed, 2022-11-23 at 13:44 +0100, Roberto Sassu wrote:
> On Wed, 2022-11-23 at 07:28 -0500, Mimi Zohar wrote:
> > Hi Roberto,
> >
> > On Wed, 2022-11-23 at 10:51 +0100, Roberto Sassu wrote:
> > > From: Roberto Sassu <roberto.sassu@xxxxxxxxxx>
> > >
> > > One of the challenges that must be tackled to move IMA and EVM to the LSM
> > > infrastructure is to ensure that EVM is capable to correctly handle
> > > multiple stacked LSMs providing an xattr at file creation. At the moment,
> > > there are few issues that would prevent a correct integration. This patch
> > > set aims at solving them.
> >
> > Let's take a step back and understand the purpose of this patch set.
> > Regardless of whether IMA and EVM are moved to the "LSM
> > infrastructure", EVM needs to support per LSM xattrs. A side affect is
> > the removal of the security_old_inode_init_security hook. This patch
> > set cover letter and patch descriptions should be limited to EVM
> > support for per LSM (multiple) xattrs. The motivation, concerns, and
> > problems of making IMA and EVM LSMs will be documented in the patch set
> > that actual makes them LSMs. Please remove all references to "move IMA
> > and EVM to the LSM infrastructure".
>
> Hi Mimi
>
> ok, will do.
>
> > When EVM was upstreamed, there were filesystem limitations on the
> > number and size of the extended attributes. In addition there were
> > performance concerns, which resulted in staging the LSM, IMA and EVM
> > xattrs, before calling initxattrs to write them at the same time. With
> > this patch set, not only are per LSM xattrs supported, but multiple per
> > LSM xattrs are supported as well. Have the size limitation concerns
> > been addressed by the different filesystems? If not, then at minimum
> > this patch set needs to at least mention it and the possible
> > ramifications.
>
> With your patch, 9d8f13ba3f483 ("security: new
> security_inode_init_security API adds function callback") you made it
> possible to set multiple xattrs at inode creation time.
True, and even then there were concerns.
> This patch set pushes further to the limits, as there could be more
> xattrs to be added to the inode. I will mention that.
Thanks
> If there are too many xattrs, I guess the only solution would be to use
> less LSMs, or a different filesystem. The per filesystem limit could be
> increased separately case by case.
Agreed, but unless it is documented somewhere, nobody but us will know
there is a potential problem. At least document it here in the cover
letter, which we'll include in the merge message.
FYI, the xattr.7 man page contains a section "Filesystem differences".
--
thanks,
Mimi
