On 3/6/2020 6:14 PM, Paul Moore wrote: > On Fri, Feb 14, 2020 at 6:45 PM 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> >> --- >> include/net/netlabel.h | 8 ++-- >> net/ipv4/cipso_ipv4.c | 23 +++++++----- >> net/netlabel/netlabel_kapi.c | 6 +-- >> net/netlabel/netlabel_unlabeled.c | 57 +++++++++++------------------ >> 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, 59 insertions(+), 62 deletions(-) > ... > > >> diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c >> index 376882215919..adb9dffc3952 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)) > This isn't as bad as the sk_buff.cb limitation so I'm not going to > worry too much about it, but just to be safe I think we should put a > compile-time check to ensure that the local tag is within the bounds > of the IPv4 option limit. If we don't put a check I worry that there > is a chance someone could get a very rude surprise at some point in > the future (yes, this is highly unlikely, but still possible). I hadn't thought about it, but that's a splendid idea. >> /* >> * Helper Functions >> @@ -1467,7 +1469,8 @@ 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; >> + memcpy(&buffer[2], &secattr->attr.lsmblob, >> + sizeof(secattr->attr.lsmblob)); >> >> return CIPSO_V4_TAG_LOC_BLEN; >> }
