On 9/16/19 11:42 AM, Stephen Smalley wrote:
> On 8/29/19 7:29 PM, Casey Schaufler wrote:
>> Move management of the sock->sk_security blob out
>> of the individual security modules and into the security
>> infrastructure. Instead of allocating the blobs from within
>> the modules the modules tell the infrastructure how much
>> space is required, and the space is allocated there.
>>
>> Reviewed-by: Kees Cook <keescook@xxxxxxxxxxxx>
>> Reviewed-by: John Johansen <john.johansen@xxxxxxxxxxxxx>
>> Signed-off-by: Casey Schaufler <casey@xxxxxxxxxxxxxxxx>
>
> One oddity noted below, but it isn't introduced by this patch so you can add my:
>
> Reviewed-by: Stephen Smalley <sds@xxxxxxxxxxxxx>
>
>> ---
>> include/linux/lsm_hooks.h | 1 +
>> security/apparmor/include/net.h | 6 ++-
>> security/apparmor/lsm.c | 38 ++++-----------
>> security/security.c | 36 +++++++++++++-
>> security/selinux/hooks.c | 78 +++++++++++++++----------------
>> security/selinux/include/objsec.h | 5 ++
>> security/selinux/netlabel.c | 23 ++++-----
>> security/smack/smack.h | 5 ++
>> security/smack/smack_lsm.c | 64 ++++++++++++-------------
>> security/smack/smack_netfilter.c | 8 ++--
>> 10 files changed, 144 insertions(+), 120 deletions(-)
>>
>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>> index f9222a04968d..b353482ea348 100644
>> --- a/include/linux/lsm_hooks.h
>> +++ b/include/linux/lsm_hooks.h
>> @@ -2047,6 +2047,7 @@ struct lsm_blob_sizes {
>> int lbs_cred;
>> int lbs_file;
>> int lbs_inode;
>> + int lbs_sock;
>> int lbs_superblock;
>> int lbs_ipc;
>> int lbs_msg_msg;
>> diff --git a/security/apparmor/include/net.h b/security/apparmor/include/net.h
>> index 7334ac966d01..adac04e3b3cc 100644
>> --- a/security/apparmor/include/net.h
>> +++ b/security/apparmor/include/net.h
>> @@ -55,7 +55,11 @@ struct aa_sk_ctx {
>> struct aa_label *peer;
>> };
>> -#define SK_CTX(X) ((X)->sk_security)
>> +static inline struct aa_sk_ctx *aa_sock(const struct sock *sk)
>> +{
>> + return sk->sk_security + apparmor_blob_sizes.lbs_sock;
>> +}
>> +
>> #define SOCK_ctx(X) SOCK_INODE(X)->i_security
>
> This use of i_security looks suspicious, but SOCK_ctx doesn't appear to be used presently. Probably should be removed in a separate patch.
>
yes this leaked is from some in dev patches that leaked into the base socket mediation patch. I can put together a patch to remove it
