On Tue, 2021-03-23 at 23:01 +0900, Tetsuo Handa wrote:
> On 2021/03/23 22:37, Tetsuo Handa wrote:
> > On 2021/03/23 21:09, Mimi Zohar wrote:
> >> Please take a look at the newer version of this patch. Do you want to
> >> add any tags?
> >
> > Oh, I didn't know that you already posted the newer version.
> >
> >> diff --git a/security/integrity/iint.c b/security/integrity/iint.c
> >> index 1d20003243c3..0ba01847e836 100644
> >> --- a/security/integrity/iint.c
> >> +++ b/security/integrity/iint.c
> >> @@ -98,6 +98,14 @@ struct integrity_iint_cache *integrity_inode_get(struct inode *inode)
> >> struct rb_node *node, *parent = NULL;
> >> struct integrity_iint_cache *iint, *test_iint;
> >>
> >> + /*
> >> + * The integrity's "iint_cache" is initialized at security_init(),
> >> + * unless it is not included in the ordered list of LSMs enabled
> >> + * on the boot command line.
> >> + */
> >> + if (!iint_cache)
> >> + panic("%s: lsm=integrity required.\n", __func__);
> >> +
> >
> > This looks strange. If "lsm=" parameter must include "integrity",
> > it implies that nobody is allowed to disable "integrity" at boot.
Integrity isn't always required. Only when something tries to use it,
does it need to be enabled. Since both integrity and the integrity
caller are runtime dependent, it is up to the user/admin to specify
"integrity" as an "lsm=" option.
> > Then, why not unconditionally call integrity_iintcache_init() by
> > not counting on DEFINE_LSM(integrity) declaration?
Initially I also questioned making "integrity" an LSM. Perhaps it's
time to reconsider. For now, it makes sense to just fix the NULL
pointer dereferencing.
Mimi
