Re: [PATCH] evm: Correct inode_init_security hooks behaviors

[Nicolas Bouchinet <nicolas.bouchinet@clip-os.org>]

Hi Mimi,

On Tue, Oct 25, 2022 at 11:58:40AM -0400, Mimi Zohar wrote:
> On Tue, 2022-10-25 at 08:06 -0700, Casey Schaufler wrote:
> > On 10/25/2022 7:21 AM, Mimi Zohar wrote:
> > > On Tue, 2022-10-25 at 15:33 +0200, Nicolas Bouchinet wrote:
> > >>> Agreed, independently as to whether BPF defines a security xattr, if
> > >>> two LSMs initialize security xattrs, then this change is needed.  Are
> > >>> there any other examples?
> > >> I think that in its current state the kernel cannot load two LSM capable of xattr
> > >> initialization as they are all defined with the `LSM_FLAG_EXCLUSIVE` flag set.
> > >> But I may be unaware of other LSM in development stage.
> > > Casey, Paul, can we get confirmation on this?
> > 
> > I'm working really hard to eliminate LSM_FLAG_EXCLUSIVE. Dealing with
> > multiple security modules initializing security xattrs has been in the
> > stacking patch sets that have been in review for years now. So no,
> > you can't wave the problem away by pointing at LSM_FLAG_EXCLUSIVE.
> 
> Please note that the original problem being addressed by this patch
> will be addressed by Roberto's BPF patch.   The question here was
> whether this addresses an existing bug, other than BPF, or a future
> one, and whether it needs to be backported.
> 
Should I split the NULL pointer dereference fix in a separated patch for EVM ?
> From your response, initializing multiple security xattrs is not an
> issue at the moment so it doesn't need to be backported.  Whether this
> patch should be upstreamed with the LSM stacking patch set is a
> separate question.
> 
> > 
> > >>> (nit: I understand the line size has generally been relaxed, but for
> > >>> IMA/EVM I would prefer it to be remain as 80 chars.)
> > >>>
> > >> No problem, will change it !
> > >>
> > >> I'll take time to run few tests with BPF and send a patch v3 with new changes.
> > > Since Roberto's patches will address the BPF bug(s), is this a fix for
> > > a real bug or a possbile future one.   Cc'ing stable might not be
> > > necessary.
Ok, will remove stable.

Thanks,

Nicolas Bouchinet