On Wed, Aug 26, 2020 at 11:21 AM Casey Schaufler <casey@xxxxxxxxxxxxxxxx> wrote: > > Netlabel uses LSM interfaces requiring an lsmblob and > the internal storage is used to pass information between > these interfaces, so change the internal data from a secid > to a lsmblob. Update the netlabel interfaces and their > callers to accommodate the change. This requires that the > modules using netlabel use the lsm_id.slot to access the > correct secid when using netlabel. > > Reviewed-by: Kees Cook <keescook@xxxxxxxxxxxx> > Reviewed-by: John Johansen <john.johansen@xxxxxxxxxxxxx> > Acked-by: Stephen Smalley <sds@xxxxxxxxxxxxx> > Signed-off-by: Casey Schaufler <casey@xxxxxxxxxxxxxxxx> > Cc: netdev@xxxxxxxxxxxxxxx > --- > include/net/netlabel.h | 8 +-- > net/ipv4/cipso_ipv4.c | 27 ++++++---- > net/netlabel/netlabel_kapi.c | 6 +-- > net/netlabel/netlabel_unlabeled.c | 79 +++++++++-------------------- > net/netlabel/netlabel_unlabeled.h | 2 +- > security/selinux/hooks.c | 2 +- > security/selinux/include/security.h | 1 + > security/selinux/netlabel.c | 2 +- > security/selinux/ss/services.c | 4 +- > security/smack/smack.h | 1 + > security/smack/smack_lsm.c | 5 +- > security/smack/smackfs.c | 10 ++-- > 12 files changed, 65 insertions(+), 82 deletions(-) Minor change suggested to a comment below, but looks good otherwise. Acked-by: Paul Moore <paul@xxxxxxxxxxxxxx> > diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c > index 2eb71579f4d2..8182b923e802 100644 > --- a/net/ipv4/cipso_ipv4.c > +++ b/net/ipv4/cipso_ipv4.c > @@ -106,15 +106,17 @@ int cipso_v4_rbm_strictvalid = 1; > /* Base length of the local tag (non-standard tag). > * Tag definition (may change between kernel versions) > * > - * 0 8 16 24 32 > - * +----------+----------+----------+----------+ > - * | 10000000 | 00000110 | 32-bit secid value | > - * +----------+----------+----------+----------+ > - * | in (host byte order)| > - * +----------+----------+ > - * > + * 0 8 16 16 + sizeof(struct lsmblob) > + * +----------+----------+---------------------+ > + * | 10000000 | 00000110 | LSM blob data | > + * +----------+----------+---------------------+ > + * > + * All secid and flag fields are in host byte order. > + * The lsmblob structure size varies depending on which > + * Linux security modules are built in the kernel. > + * The data is opaque. > */ > -#define CIPSO_V4_TAG_LOC_BLEN 6 > +#define CIPSO_V4_TAG_LOC_BLEN (2 + sizeof(struct lsmblob)) > > /* > * Helper Functions > @@ -1469,7 +1471,12 @@ static int cipso_v4_gentag_loc(const struct cipso_v4_doi *doi_def, > > buffer[0] = CIPSO_V4_TAG_LOCAL; > buffer[1] = CIPSO_V4_TAG_LOC_BLEN; > - *(u32 *)&buffer[2] = secattr->attr.secid; > + /* Ensure that there is sufficient space in the CIPSO header > + * for the LSM data. This should never become an issue. > + * The check is made from an abundance of caution. */ > + BUILD_BUG_ON(CIPSO_V4_TAG_LOC_BLEN > CIPSO_V4_OPT_LEN_MAX); I like the BUILD_BUG_ON() check, but for reasons very similar to the unix_skb_params changes I don't really like the "should never become an issue" commentary. Granted, it is unlikely, but I don't think it is wise to thumb our nose at fate. > + memcpy(&buffer[2], &secattr->attr.lsmblob, > + sizeof(secattr->attr.lsmblob)); > > return CIPSO_V4_TAG_LOC_BLEN; > } -- paul moore www.paul-moore.com
