On Mon, Jun 24, 2019 at 09:39:05AM -0700, Casey Schaufler wrote: > On 6/22/2019 3:48 PM, Kees Cook wrote: > > On Fri, Jun 21, 2019 at 11:52:19AM -0700, Casey Schaufler wrote: > >> + struct security_hook_list *hp; > >> + > >> + lsmblob_init(blob, 0); > >> + hlist_for_each_entry(hp, &security_hook_heads.ipc_getsecid, list) > >> + hp->hook.ipc_getsecid(ipcp, &blob->secid[hp->slot]); > > Just for sanity when using hp->slot, it might be good to do something > > like this in the places it gets used. Like for here: > > > > if (!WARN_ON(hp->slot < 0 || hp->slot >= LSMBLOB_COUNT)) > > hp->hook.ipc_getsecid(ipcp, &blob->secid[hp->slot]); > > > > This _should_ be overkill, but since lists of hooks that trigger slot > > assignment is hardcoded, it seems nice to cover any future problems or > > mismatches. > > How about a CONFIG_LSM_SLOT_CHECK around a function lsm_slot_check()? > If configured, it does the WARN_ON, and if not it's a static inline > true return. As you say, it's probably overkill, but it would be available > for the paranoid/debug/bringup situation. No, this doesn't need another CONFIG. The test is nearly free and WARN means it's wrapped in an unlikely already. I think just adding it outright would be fine. -- Kees Cook
