* Re: [PATCH v4 00/23] LSM: Module stacking for AppArmor
From: Kees Cook @ 2019-06-27 17:07 UTC (permalink / raw)
To: James Morris
Cc: Casey Schaufler, casey.schaufler, linux-security-module, selinux,
John Johansen, Tetsuo Handa, Paul Moore, Stephen Smalley
In-Reply-To: <alpine.LRH.2.21.1906271409460.16512@namei.org>
On Thu, Jun 27, 2019 at 02:10:18PM +1000, James Morris wrote:
> On Thu, 27 Jun 2019, James Morris wrote:
>
> > Confirming there's no oops when Tomoyo is un-selected in the kernel
> > config.
>
> n/m, the problem is still there.
Were you able to test my fix for this? I wonder if what I found was just
a coincidence.
--
Kees Cook
^ permalink raw reply
* Re: [PATCH v4 17/23] LSM: Use lsmcontext in security_secid_to_secctx
From: Kees Cook @ 2019-06-27 17:17 UTC (permalink / raw)
To: Casey Schaufler
Cc: casey.schaufler, jmorris, linux-security-module, selinux,
john.johansen, penguin-kernel, paul, sds
In-Reply-To: <cacf9eda-7486-80d3-f47d-b0dbb6872091@schaufler-ca.com>
On Thu, Jun 27, 2019 at 09:29:25AM -0700, Casey Schaufler wrote:
> On 6/26/2019 8:53 PM, Kees Cook wrote:
> > On Wed, Jun 26, 2019 at 12:22:28PM -0700, Casey Schaufler wrote:
> >> Replace the (secctx,seclen) pointer pair with a single
> >> lsmcontext pointer to allow return of the LSM identifier
> >> along with the context and context length. This allows
> >> security_release_secctx() to know how to release the
> >> context. Callers have been modified to use or save the
> >> returned data from the new structure.
> >>
> >> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> >> ---
> >> drivers/android/binder.c | 24 ++++++---------
> >> include/linux/security.h | 4 +--
> >> include/net/scm.h | 9 ++----
> >> kernel/audit.c | 29 +++++++-----------
> >> kernel/auditsc.c | 31 +++++++------------
> >> net/ipv4/ip_sockglue.c | 7 ++---
> >> net/netfilter/nf_conntrack_netlink.c | 14 +++++----
> >> net/netfilter/nf_conntrack_standalone.c | 7 ++---
> >> net/netfilter/nfnetlink_queue.c | 5 +++-
> >> net/netlabel/netlabel_unlabeled.c | 40 ++++++++-----------------
> >> net/netlabel/netlabel_user.c | 7 ++---
> >> security/security.c | 9 ++++--
> >> 12 files changed, 72 insertions(+), 114 deletions(-)
> >>
> >> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> >> index 89e574be34cc..5d417a7b9bb3 100644
> >> --- a/drivers/android/binder.c
> >> +++ b/drivers/android/binder.c
> >> @@ -2874,9 +2874,7 @@ static void binder_transaction(struct binder_proc *proc,
> >> binder_size_t last_fixup_min_off = 0;
> >> struct binder_context *context = proc->context;
> >> int t_debug_id = atomic_inc_return(&binder_last_id);
> >> - char *secctx = NULL;
> >> - u32 secctx_sz = 0;
> >> - struct lsmcontext scaff; /* scaffolding */
> >> + struct lsmcontext lsmctx;
> > As James found, this needs to be zero initialized:
> >
> > struct lsmcontext lsmctx = { };
>
> Thanks! I'll incorporate this in v5. It's great to
> have y'all checking it out.
I looked through other removed NULL assignments, and I think I see some
other issues...
binder_alloc_copy_to_buffer(&target_proc->alloc,
t->buffer, buf_offset,
- secctx, secctx_sz);
- security_release_secctx(secctx, secctx_sz);
- secctx = NULL;
+ lsmctx.context, lsmctx.len);
+ security_release_secctx(&lsmctx);
The new security_release_secctx() performs the zeroing if there is a
slot match... should it be unconditional? (And should the no-op version
also zero?)
@@ -2420,8 +2420,7 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct
svc_fh *fhp,
__be32 status;
int err;
struct nfs4_acl *acl = NULL;
- void *context = NULL;
- int contextlen;
+ struct lsmcontext context;
This needs the same = { } to retain the same meaning.
And here:
@@ -1191,8 +1191,8 @@ static int audit_receive_msg(struct sk_buff *skb,
struct nlmsghdr *nlh)
struct audit_buffer *ab;
u16 msg_type = nlh->nlmsg_type;
struct audit_sig_info *sig_data;
- char *ctx = NULL;
u32 len;
+ struct lsmcontext context;
And here:
@@ -2069,24 +2070,23 @@ void audit_log_key(struct audit_buffer *ab, char
*key)
int audit_log_task_context(struct audit_buffer *ab)
{
- char *ctx = NULL;
- unsigned len;
int error;
- u32 sid;
+ struct lsmblob le;
+ struct lsmcontext context;
and here:
static int audit_log_pid_context(struct audit_context *context, pid_t
pid,
- kuid_t auid, kuid_t uid, unsigned int
sessionid,
- u32 sid, char *comm)
+ kuid_t auid, kuid_t uid,
+ unsigned int sessionid,
+ struct lsmblob *l, char *comm)
{
struct audit_buffer *ab;
- char *ctx = NULL;
- u32 len;
+ struct lsmcontext lsmctx;
Maybe here?
- if (osid) {
- char *ctx = NULL;
- u32 len;
- if (security_secid_to_secctx(osid, &ctx, &len))
{
- audit_log_format(ab, " osid=%u", osid);
+ if (lsmblob_is_set(olsm)) {
+ struct lsmcontext lsmcxt;
+ if (security_secid_to_secctx(olsm, &lsmcxt))
*call_panic = 1;
and:
- if (n->osid != 0) {
- char *ctx = NULL;
- u32 len;
+ if (lsmblob_is_set(&n->olsm)) {
+ struct lsmcontext lsmctx;
and:
@@ -395,7 +401,7 @@ nfqnl_build_packet_message(struct net *net, struct
nfqnl_instance *queue,
enum ip_conntrack_info uninitialized_var(ctinfo);
struct nfnl_ct_hook *nfnl_ct;
bool csum_verify;
- char *secdata = NULL;
+ struct lsmcontext context;
and:
@@ -387,8 +387,7 @@ int netlbl_unlhsh_add(struct net *net,
struct net_device *dev;
struct netlbl_unlhsh_iface *iface;
struct audit_buffer *audit_buf = NULL;
- char *secctx = NULL;
- u32 secctx_len;
+ struct lsmcontext context;
Sorry I forgot to check those the first time through!
--
Kees Cook
^ permalink raw reply
* Re: [PATCH v4 17/23] LSM: Use lsmcontext in security_secid_to_secctx
From: Casey Schaufler @ 2019-06-27 17:36 UTC (permalink / raw)
To: Kees Cook
Cc: casey.schaufler, jmorris, linux-security-module, selinux,
john.johansen, penguin-kernel, paul, sds, casey
In-Reply-To: <201906271007.4248A3AE@keescook>
On 6/27/2019 10:17 AM, Kees Cook wrote:
> On Thu, Jun 27, 2019 at 09:29:25AM -0700, Casey Schaufler wrote:
>> On 6/26/2019 8:53 PM, Kees Cook wrote:
>>> On Wed, Jun 26, 2019 at 12:22:28PM -0700, Casey Schaufler wrote:
>>>> Replace the (secctx,seclen) pointer pair with a single
>>>> lsmcontext pointer to allow return of the LSM identifier
>>>> along with the context and context length. This allows
>>>> security_release_secctx() to know how to release the
>>>> context. Callers have been modified to use or save the
>>>> returned data from the new structure.
>>>>
>>>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>>>> ---
>>>> drivers/android/binder.c | 24 ++++++---------
>>>> include/linux/security.h | 4 +--
>>>> include/net/scm.h | 9 ++----
>>>> kernel/audit.c | 29 +++++++-----------
>>>> kernel/auditsc.c | 31 +++++++------------
>>>> net/ipv4/ip_sockglue.c | 7 ++---
>>>> net/netfilter/nf_conntrack_netlink.c | 14 +++++----
>>>> net/netfilter/nf_conntrack_standalone.c | 7 ++---
>>>> net/netfilter/nfnetlink_queue.c | 5 +++-
>>>> net/netlabel/netlabel_unlabeled.c | 40 ++++++++-----------------
>>>> net/netlabel/netlabel_user.c | 7 ++---
>>>> security/security.c | 9 ++++--
>>>> 12 files changed, 72 insertions(+), 114 deletions(-)
>>>>
>>>> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
>>>> index 89e574be34cc..5d417a7b9bb3 100644
>>>> --- a/drivers/android/binder.c
>>>> +++ b/drivers/android/binder.c
>>>> @@ -2874,9 +2874,7 @@ static void binder_transaction(struct binder_proc *proc,
>>>> binder_size_t last_fixup_min_off = 0;
>>>> struct binder_context *context = proc->context;
>>>> int t_debug_id = atomic_inc_return(&binder_last_id);
>>>> - char *secctx = NULL;
>>>> - u32 secctx_sz = 0;
>>>> - struct lsmcontext scaff; /* scaffolding */
>>>> + struct lsmcontext lsmctx;
>>> As James found, this needs to be zero initialized:
>>>
>>> struct lsmcontext lsmctx = { };
>> Thanks! I'll incorporate this in v5. It's great to
>> have y'all checking it out.
> I looked through other removed NULL assignments, and I think I see some
> other issues...
>
> binder_alloc_copy_to_buffer(&target_proc->alloc,
> t->buffer, buf_offset,
> - secctx, secctx_sz);
> - security_release_secctx(secctx, secctx_sz);
> - secctx = NULL;
> + lsmctx.context, lsmctx.len);
> + security_release_secctx(&lsmctx);
>
> The new security_release_secctx() performs the zeroing if there is a
> slot match... should it be unconditional? (And should the no-op version
> also zero?)
I don't see a reason not to zero in all cases. I'll change that.
>
> @@ -2420,8 +2420,7 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct
> svc_fh *fhp,
> __be32 status;
> int err;
> struct nfs4_acl *acl = NULL;
> - void *context = NULL;
> - int contextlen;
> + struct lsmcontext context;
>
> This needs the same = { } to retain the same meaning.
I'm going to have security_secid_to_secctx() initialize
the lsmcontext. There's no case where it should be used
to update an existing context, and it makes more sense to
do it in one place than to make all the callers worry
about it.
> And here:
>
> @@ -1191,8 +1191,8 @@ static int audit_receive_msg(struct sk_buff *skb,
> struct nlmsghdr *nlh)
> struct audit_buffer *ab;
> u16 msg_type = nlh->nlmsg_type;
> struct audit_sig_info *sig_data;
> - char *ctx = NULL;
> u32 len;
> + struct lsmcontext context;
>
> And here:
>
> @@ -2069,24 +2070,23 @@ void audit_log_key(struct audit_buffer *ab, char
> *key)
>
> int audit_log_task_context(struct audit_buffer *ab)
> {
> - char *ctx = NULL;
> - unsigned len;
> int error;
> - u32 sid;
> + struct lsmblob le;
> + struct lsmcontext context;
>
> and here:
>
> static int audit_log_pid_context(struct audit_context *context, pid_t
> pid,
> - kuid_t auid, kuid_t uid, unsigned int
> sessionid,
> - u32 sid, char *comm)
> + kuid_t auid, kuid_t uid,
> + unsigned int sessionid,
> + struct lsmblob *l, char *comm)
> {
> struct audit_buffer *ab;
> - char *ctx = NULL;
> - u32 len;
> + struct lsmcontext lsmctx;
>
> Maybe here?
>
> - if (osid) {
> - char *ctx = NULL;
> - u32 len;
> - if (security_secid_to_secctx(osid, &ctx, &len))
> {
> - audit_log_format(ab, " osid=%u", osid);
> + if (lsmblob_is_set(olsm)) {
> + struct lsmcontext lsmcxt;
> + if (security_secid_to_secctx(olsm, &lsmcxt))
> *call_panic = 1;
>
> and:
>
> - if (n->osid != 0) {
> - char *ctx = NULL;
> - u32 len;
> + if (lsmblob_is_set(&n->olsm)) {
> + struct lsmcontext lsmctx;
>
>
> and:
>
> @@ -395,7 +401,7 @@ nfqnl_build_packet_message(struct net *net, struct
> nfqnl_instance *queue,
> enum ip_conntrack_info uninitialized_var(ctinfo);
> struct nfnl_ct_hook *nfnl_ct;
> bool csum_verify;
> - char *secdata = NULL;
> + struct lsmcontext context;
>
> and:
>
> @@ -387,8 +387,7 @@ int netlbl_unlhsh_add(struct net *net,
> struct net_device *dev;
> struct netlbl_unlhsh_iface *iface;
> struct audit_buffer *audit_buf = NULL;
> - char *secctx = NULL;
> - u32 secctx_len;
> + struct lsmcontext context;
>
>
> Sorry I forgot to check those the first time through!
>
^ permalink raw reply
* Re: [PATCH v4 04/23] LSM: Create and manage the lsmblob data structure.
From: Casey Schaufler @ 2019-06-27 17:43 UTC (permalink / raw)
To: John Johansen, casey.schaufler, jmorris, linux-security-module,
selinux
Cc: keescook, penguin-kernel, paul, sds
In-Reply-To: <19468cf8-8702-b391-fae5-197ddbb6da4d@canonical.com>
On 6/26/2019 4:39 PM, John Johansen wrote:
> On 6/26/19 12:22 PM, Casey Schaufler wrote:
>> When more than one security module is exporting data to
>> audit and networking sub-systems a single 32 bit integer
>> is no longer sufficient to represent the data. Add a
>> structure to be used instead.
>>
>> The lsmblob structure is currently an array of
>> u32 "secids". There is an entry for each of the
>> security modules built into the system that would
>> use secids if active. The system assigns the module
>> a "slot" when it registers hooks. If modules are
>> compiled in but not registered there will be unused
>> slots.
>>
>> A new lsm_id structure, which contains the name
>> of the LSM and its slot number, is created. There
>> is an instance for each LSM, which assigns the name
>> and passes it to the infrastructure to set the slot.
>>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>> ---
>> include/linux/lsm_hooks.h | 12 +++++--
>> include/linux/security.h | 66 ++++++++++++++++++++++++++++++++++++++
>> security/apparmor/lsm.c | 4 ++-
>> security/commoncap.c | 7 +++-
>> security/loadpin/loadpin.c | 8 ++++-
>> security/safesetid/lsm.c | 8 ++++-
>> security/security.c | 31 ++++++++++++++----
>> security/selinux/hooks.c | 5 ++-
>> security/smack/smack_lsm.c | 4 ++-
>> security/tomoyo/tomoyo.c | 8 ++++-
>> security/yama/yama_lsm.c | 4 ++-
>> 11 files changed, 140 insertions(+), 17 deletions(-)
>>
>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>> index 3fe39abccc8f..fe1fb7a69ee5 100644
>> --- a/include/linux/lsm_hooks.h
>> +++ b/include/linux/lsm_hooks.h
>> @@ -2029,6 +2029,14 @@ struct security_hook_heads {
>> #endif /* CONFIG_BPF_SYSCALL */
>> } __randomize_layout;
>>
>> +/*
>> + * Information that identifies a security module.
>> + */
>> +struct lsm_id {
>> + const char *lsm; /* Name of the LSM */
>> + int slot; /* Slot in lsmblob if one is allocated */
>> +};
>> +
>> /*
>> * Security module hook list structure.
>> * For use with generic list macros for common operations.
>> @@ -2037,7 +2045,7 @@ struct security_hook_list {
>> struct hlist_node list;
>> struct hlist_head *head;
>> union security_list_options hook;
>> - char *lsm;
>> + struct lsm_id *lsmid;
>> } __randomize_layout;
>>
>> /*
>> @@ -2068,7 +2076,7 @@ extern struct security_hook_heads security_hook_heads;
>> extern char *lsm_names;
>>
>> extern void security_add_hooks(struct security_hook_list *hooks, int count,
>> - char *lsm);
>> + struct lsm_id *lsmid);
>>
>> #define LSM_FLAG_LEGACY_MAJOR BIT(0)
>> #define LSM_FLAG_EXCLUSIVE BIT(1)
>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index 49f2685324b0..5bb8b9a6fa84 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -76,6 +76,72 @@ enum lsm_event {
>> LSM_POLICY_CHANGE,
>> };
>>
>> +/*
>> + * Data exported by the security modules
>> + *
>> + * Any LSM that provides secid or secctx based hooks must be included.
>> + */
>> +#define LSMBLOB_ENTRIES ( \
>> + (IS_ENABLED(CONFIG_SECURITY_SELINUX) ? 1 : 0) + \
>> + (IS_ENABLED(CONFIG_SECURITY_SMACK) ? 1 : 0) + \
>> + (IS_ENABLED(CONFIG_SECURITY_APPARMOR) ? 1 : 0))
>> +
>> +struct lsmblob {
>> + u32 secid[LSMBLOB_ENTRIES];
>> +};
>> +
>> +#define LSMBLOB_INVALID -1 /* Not a valid LSM slot number */
>> +#define LSMBLOB_NEEDED -2 /* Slot requested on initialization */
>> +#define LSMBLOB_NOT_NEEDED -3 /* Slot not requested */
>> +
>> +/**
>> + * lsmblob_init - initialize an lsmblob structure.
>> + * @blob: Pointer to the data to initialize
>> + * @secid: The initial secid value
>> + *
>> + * Set all secid for all modules to the specified value.
>> + */
>> +static inline void lsmblob_init(struct lsmblob *blob, u32 secid)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < LSMBLOB_ENTRIES; i++)
>> + blob->secid[i] = secid;
>> +}
>> +
>> +/**
>> + * lsmblob_is_set - report if there is an value in the lsmblob
>> + * @blob: Pointer to the exported LSM data
>> + *
>> + * Returns true if there is a secid set, false otherwise
>> + */
>> +static inline bool lsmblob_is_set(struct lsmblob *blob)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < LSMBLOB_ENTRIES; i++)
>> + if (blob->secid[i] != 0)
>> + return true;
>> + return false;
>> +}
>> +
>> +/**
>> + * lsmblob_equal - report if the two lsmblob's are equal
>> + * @bloba: Pointer to one LSM data
>> + * @blobb: Pointer to the other LSM data
>> + *
>> + * Returns true if all entries in the two are equal, false otherwise
>> + */
>> +static inline bool lsmblob_equal(struct lsmblob *bloba, struct lsmblob *blobb)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < LSMBLOB_ENTRIES; i++)
>> + if (bloba->secid[i] != blobb->secid[i])
>> + return false;
>> + return true;
>> +}
>> +
>> /* These functions are in security/commoncap.c */
>> extern int cap_capable(const struct cred *cred, struct user_namespace *ns,
>> int cap, unsigned int opts);
>> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
>> index 2716e7731279..6d2eefc9b7c1 100644
>> --- a/security/apparmor/lsm.c
>> +++ b/security/apparmor/lsm.c
>> @@ -1138,6 +1138,8 @@ struct lsm_blob_sizes apparmor_blob_sizes __lsm_ro_after_init = {
>> .lbs_sock = sizeof(struct aa_sk_ctx),
>> };
>>
>> +static struct lsm_id apparmor_lsmid = { .lsm="apparmor", .slot=LSMBLOB_NEEDED };
> __lsm_ro_after_init
>
>
>> +
>> static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = {
>> LSM_HOOK_INIT(ptrace_access_check, apparmor_ptrace_access_check),
>> LSM_HOOK_INIT(ptrace_traceme, apparmor_ptrace_traceme),
>> @@ -1679,7 +1681,7 @@ static int __init apparmor_init(void)
>> goto buffers_out;
>> }
>> security_add_hooks(apparmor_hooks, ARRAY_SIZE(apparmor_hooks),
>> - "apparmor");
>> + &apparmor_lsmid);
>>
>> /* Report that AppArmor successfully initialized */
>> apparmor_initialized = 1;
>> diff --git a/security/commoncap.c b/security/commoncap.c
>> index afd9679ca866..305a6088c81e 100644
>> --- a/security/commoncap.c
>> +++ b/security/commoncap.c
>> @@ -1344,6 +1344,11 @@ int cap_mmap_file(struct file *file, unsigned long reqprot,
>>
>> #ifdef CONFIG_SECURITY
>>
>> +static struct lsm_id capability_lsmid = {
> __lsm_ro_after_init
>
>> + .lsm="capability",
>> + .slot=LSMBLOB_NOT_NEEDED
>> +};
>> +
>> static struct security_hook_list capability_hooks[] __lsm_ro_after_init = {
>> LSM_HOOK_INIT(capable, cap_capable),
>> LSM_HOOK_INIT(settime, cap_settime),
>> @@ -1368,7 +1373,7 @@ static struct security_hook_list capability_hooks[] __lsm_ro_after_init = {
>> static int __init capability_init(void)
>> {
>> security_add_hooks(capability_hooks, ARRAY_SIZE(capability_hooks),
>> - "capability");
>> + &capability_lsmid);
>> return 0;
>> }
>>
>> diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
>> index 055fb0a64169..13db59d5327e 100644
>> --- a/security/loadpin/loadpin.c
>> +++ b/security/loadpin/loadpin.c
>> @@ -181,6 +181,11 @@ static int loadpin_load_data(enum kernel_load_data_id id)
>> return loadpin_read_file(NULL, (enum kernel_read_file_id) id);
>> }
>>
>> +static struct lsm_id loadpin_lsmid = {
> __lsm_ro_after_init
>
>
>> + .lsm="loadpin",
>> + .slot=LSMBLOB_NOT_NEEDED
>> +};
>> +
>> static struct security_hook_list loadpin_hooks[] __lsm_ro_after_init = {
>> LSM_HOOK_INIT(sb_free_security, loadpin_sb_free_security),
>> LSM_HOOK_INIT(kernel_read_file, loadpin_read_file),
>> @@ -191,7 +196,8 @@ static int __init loadpin_init(void)
>> {
>> pr_info("ready to pin (currently %senforcing)\n",
>> enforce ? "" : "not ");
>> - security_add_hooks(loadpin_hooks, ARRAY_SIZE(loadpin_hooks), "loadpin");
>> + security_add_hooks(loadpin_hooks, ARRAY_SIZE(loadpin_hooks),
>> + &loadpin_lsmid);
>> return 0;
>> }
>>
>> diff --git a/security/safesetid/lsm.c b/security/safesetid/lsm.c
>> index cecd38e2ac80..ca34badde4cf 100644
>> --- a/security/safesetid/lsm.c
>> +++ b/security/safesetid/lsm.c
>> @@ -255,6 +255,11 @@ void flush_safesetid_whitelist_entries(void)
>> }
>> }
>>
>> +static struct lsm_id safesetid_lsmid = {
> __lsm_ro_after_init
>
>
>> + .lsm="safesetid",
>> + .slot=LSMBLOB_NOT_NEEDED
>> +};
>> +
>> static struct security_hook_list safesetid_security_hooks[] = {
>> LSM_HOOK_INIT(task_fix_setuid, safesetid_task_fix_setuid),
>> LSM_HOOK_INIT(capable, safesetid_security_capable)
>> @@ -263,7 +268,8 @@ static struct security_hook_list safesetid_security_hooks[] = {
>> static int __init safesetid_security_init(void)
>> {
>> security_add_hooks(safesetid_security_hooks,
>> - ARRAY_SIZE(safesetid_security_hooks), "safesetid");
>> + ARRAY_SIZE(safesetid_security_hooks),
>> + &safesetid_lsmid);
>>
>> /* Report that SafeSetID successfully initialized */
>> safesetid_initialized = 1;
>> diff --git a/security/security.c b/security/security.c
>> index 7cfedb90210a..27e2db3d6b04 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -317,6 +317,7 @@ static void __init ordered_lsm_init(void)
>> init_debug("sock blob size = %d\n", blob_sizes.lbs_sock);
>> init_debug("superblock blob size = %d\n", blob_sizes.lbs_superblock);
>> init_debug("task blob size = %d\n", blob_sizes.lbs_task);
>> + init_debug("lsmblob size = %lu\n", sizeof(struct lsmblob));
>>
>> /*
>> * Create any kmem_caches needed for blobs
>> @@ -399,7 +400,7 @@ static bool match_last_lsm(const char *list, const char *lsm)
>> return !strcmp(last, lsm);
>> }
>>
>> -static int lsm_append(char *new, char **result)
>> +static int lsm_append(const char *new, char **result)
>> {
>> char *cp;
>>
>> @@ -420,24 +421,40 @@ static int lsm_append(char *new, char **result)
>> return 0;
>> }
>>
>> +/*
>> + * Current index to use while initializing the lsmblob secid list.
>> + */
>> +static int lsm_slot __initdata;
>> +
>> /**
>> * security_add_hooks - Add a modules hooks to the hook lists.
>> * @hooks: the hooks to add
>> * @count: the number of hooks to add
>> - * @lsm: the name of the security module
>> + * @lsmid: the identification information for the security module
>> *
>> * Each LSM has to register its hooks with the infrastructure.
>> + * If the LSM is using hooks that export secids allocate a slot
>> + * for it in the lsmblob.
>> */
>> void __init security_add_hooks(struct security_hook_list *hooks, int count,
>> - char *lsm)
>> + struct lsm_id *lsmid)
>> {
>> int i;
>>
>> + if (lsmid->slot == LSMBLOB_NEEDED) {
>> + if (lsm_slot >= LSMBLOB_ENTRIES)
>> + panic("%s Too many LSMs registered.\n", __func__);
>> + lsmid->slot = lsm_slot++;
>> + init_debug("%s assigned lsmblob slot %d\n", lsmid->lsm,
>> + lsmid->slot);
>> + }
> It might be nice (but not required) to add back in the list of hooks that need
> secids as a cross check of LSMLOB_NOT_NEEDED. It would allow us to catch
> bad registrations upfront instead of crashing the kernel when the hook gets
> used.
I considered leaving that check in, but my experience with
creating the list leads me to expect it to become a headache
of the 3rd order when hooks are added or changed by anyone
who hasn't been involved in this protracted process. It's not
always obvious that a hook needs to be on the list. Hmm.
That makes me think that a bit of documentation on when to
use LSMBLOB_NEEDED vs LSMBLOB_NOT_NEEDED is needed.
>> +
>> for (i = 0; i < count; i++) {
>> - hooks[i].lsm = lsm;
>> + hooks[i].lsmid = lsmid;
>> hlist_add_tail_rcu(&hooks[i].list, hooks[i].head);
>> }
>> - if (lsm_append(lsm, &lsm_names) < 0)
>> +
>> + if (lsm_append(lsmid->lsm, &lsm_names) < 0)
>> panic("%s - Cannot get early memory.\n", __func__);
>> }
>>
>> @@ -1917,7 +1934,7 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name,
>> struct security_hook_list *hp;
>>
>> hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) {
>> - if (lsm != NULL && strcmp(lsm, hp->lsm))
>> + if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
>> continue;
>> return hp->hook.getprocattr(p, name, value);
>> }
>> @@ -1930,7 +1947,7 @@ int security_setprocattr(const char *lsm, const char *name, void *value,
>> struct security_hook_list *hp;
>>
>> hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) {
>> - if (lsm != NULL && strcmp(lsm, hp->lsm))
>> + if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
>> continue;
>> return hp->hook.setprocattr(name, value, size);
>> }
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index c83ec2652eda..8c93b07bb353 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -6622,6 +6622,8 @@ struct lsm_blob_sizes selinux_blob_sizes __lsm_ro_after_init = {
>> .lbs_superblock = sizeof(struct superblock_security_struct),
>> };
>>
>> +static struct lsm_id selinux_lsmid = { .lsm="selinux", .slot=LSMBLOB_NEEDED };
>> +
>> static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
>> LSM_HOOK_INIT(binder_set_context_mgr, selinux_binder_set_context_mgr),
>> LSM_HOOK_INIT(binder_transaction, selinux_binder_transaction),
>> @@ -6877,7 +6879,8 @@ static __init int selinux_init(void)
>>
>> hashtab_cache_init();
>>
>> - security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks), "selinux");
>> + security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks),
>> + &selinux_lsmid);
>>
>> if (avc_add_callback(selinux_netcache_avc_callback, AVC_CALLBACK_RESET))
>> panic("SELinux: Unable to register AVC netcache callback\n");
>> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
>> index e9560b078efe..ad646b865295 100644
>> --- a/security/smack/smack_lsm.c
>> +++ b/security/smack/smack_lsm.c
>> @@ -4553,6 +4553,8 @@ struct lsm_blob_sizes smack_blob_sizes __lsm_ro_after_init = {
>> .lbs_superblock = sizeof(struct superblock_smack),
>> };
>>
>> +static struct lsm_id smack_lsmid = { .lsm="smack", .slot=LSMBLOB_NEEDED };
> __lsm_ro_after_init
>
>> +
>> static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
>> LSM_HOOK_INIT(ptrace_access_check, smack_ptrace_access_check),
>> LSM_HOOK_INIT(ptrace_traceme, smack_ptrace_traceme),
>> @@ -4743,7 +4745,7 @@ static __init int smack_init(void)
>> /*
>> * Register with LSM
>> */
>> - security_add_hooks(smack_hooks, ARRAY_SIZE(smack_hooks), "smack");
>> + security_add_hooks(smack_hooks, ARRAY_SIZE(smack_hooks), &smack_lsmid);
>> smack_enabled = 1;
>>
>> pr_info("Smack: Initializing.\n");
>> diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
>> index 716c92ec941a..57e6b845ea51 100644
>> --- a/security/tomoyo/tomoyo.c
>> +++ b/security/tomoyo/tomoyo.c
>> @@ -529,6 +529,11 @@ static void tomoyo_task_free(struct task_struct *task)
>> }
>> }
>>
>> +static struct lsm_id tomoyo_lsmid = {
> __lsm_ro_after_init
>
>> + .lsm="tomoyo",
>> + .slot=LSMBLOB_NOT_NEEDED
>> +};
>> +
>> /*
>> * tomoyo_security_ops is a "struct security_operations" which is used for
>> * registering TOMOYO.
>> @@ -581,7 +586,8 @@ static int __init tomoyo_init(void)
>> struct tomoyo_task *s = tomoyo_task(current);
>>
>> /* register ourselves with the security framework */
>> - security_add_hooks(tomoyo_hooks, ARRAY_SIZE(tomoyo_hooks), "tomoyo");
>> + security_add_hooks(tomoyo_hooks, ARRAY_SIZE(tomoyo_hooks),
>> + &tomoyo_lsmid);
>> pr_info("TOMOYO Linux initialized\n");
>> s->domain_info = &tomoyo_kernel_domain;
>> atomic_inc(&tomoyo_kernel_domain.users);
>> diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c
>> index efac68556b45..2263822a4af7 100644
>> --- a/security/yama/yama_lsm.c
>> +++ b/security/yama/yama_lsm.c
>> @@ -425,6 +425,8 @@ static int yama_ptrace_traceme(struct task_struct *parent)
>> return rc;
>> }
>>
>> +static struct lsm_id yama_lsmid = { .lsm="yama", .slot=LSMBLOB_NOT_NEEDED };
> __lsm_ro_after_init
>
>> +
>> static struct security_hook_list yama_hooks[] __lsm_ro_after_init = {
>> LSM_HOOK_INIT(ptrace_access_check, yama_ptrace_access_check),
>> LSM_HOOK_INIT(ptrace_traceme, yama_ptrace_traceme),
>> @@ -482,7 +484,7 @@ static inline void yama_init_sysctl(void) { }
>> static int __init yama_init(void)
>> {
>> pr_info("Yama: becoming mindful.\n");
>> - security_add_hooks(yama_hooks, ARRAY_SIZE(yama_hooks), "yama");
>> + security_add_hooks(yama_hooks, ARRAY_SIZE(yama_hooks), &yama_lsmid);
>> yama_init_sysctl();
>> return 0;
>> }
>>
> I like the change,
>
> Reviewed-by: John Johansen <john.johansen@canonical.com>
^ permalink raw reply
* Re: [PATCH V33 24/30] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: James Morris @ 2019-06-27 18:06 UTC (permalink / raw)
To: Stephen Smalley
Cc: Andy Lutomirski, Andy Lutomirski, Matthew Garrett, linux-security,
LKML, Linux API, David Howells, Alexei Starovoitov,
Matthew Garrett, Network Development, Chun-Yi Lee,
Daniel Borkmann, linux-security-module
In-Reply-To: <bce70c8b-9efd-6362-d536-cfbbcf70b0b7@tycho.nsa.gov>
On Thu, 27 Jun 2019, Stephen Smalley wrote:
> There are two scenarios where finer-grained distinctions make sense:
>
> - Users may need to enable specific functionality that falls under the
> umbrella of "confidentiality" or "integrity" lockdown. Finer-grained lockdown
> reasons free them from having to make an all-or-nothing choice between lost
> functionality or no lockdown at all.
Agreed. This will be used for more than just UEFI secure boot on desktops,
e.g. embedded systems using verified boot, where finer grained policy will
be needed for what are sometimes very specific use-cases (which may be
also covered by other mitigations).
> This can be supported directly by the
> lockdown module without any help from SELinux or other security modules; we
> just need the ability to specify these finer-grained lockdown levels via the
> boot parameters and securityfs nodes.
If the lockdown LSM implements fine grained policy (rather than the simple
coarse grained policy), I'd suggest adding a new lockdown level of
'custom' which by default enables all hooks but allows selective
disablement via params/sysfs.
This would be simpler than telling users to use a different lockdown LSM
for this.
> - Different processes/programs may need to use different sets of functionality
> restricted via lockdown confidentiality or integrity categories. If we have
> to allow all-or-none for the set of interfaces/functionality covered by the
> generic confidentiality or integrity categories, then we'll end up having to
> choose between lost functionality or overprivileged processes, neither of
> which is optimal.
>
> Is it truly the case that everything under the "confidentiality" category
> poses the same level of risk to kernel confidentiality, and similarly for
> everything under the "integrity" category? If not, then being able to
> distinguish them definitely has benefit.
Good question. We can't know the answer to this unless we know how an
attacker might leverage access.
The value here IMHO is more in allowing tradeoffs to be made by system
designers vs. disabling lockdown entirely.
> I'm still not clear though on how/if this will compose with or be overridden
> by other security modules. We would need some means for another security
> module to take over lockdown decisions once it has initialized (including
> policy load), and to be able to access state that is currently private to the
> lockdown module, like the level.
Why not utilize stacking (restrictively), similarly to capabilities?
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH v4 00/23] LSM: Module stacking for AppArmor
From: James Morris @ 2019-06-27 18:10 UTC (permalink / raw)
To: Kees Cook
Cc: Casey Schaufler, casey.schaufler, linux-security-module, selinux,
John Johansen, Tetsuo Handa, Paul Moore, Stephen Smalley
In-Reply-To: <201906271006.57DE3C2@keescook>
On Thu, 27 Jun 2019, Kees Cook wrote:
> On Thu, Jun 27, 2019 at 02:10:18PM +1000, James Morris wrote:
> > On Thu, 27 Jun 2019, James Morris wrote:
> >
> > > Confirming there's no oops when Tomoyo is un-selected in the kernel
> > > config.
> >
> > n/m, the problem is still there.
>
> Were you able to test my fix for this? I wonder if what I found was just
> a coincidence.
Seems to have fixed the oops I was seeing.
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 4adf4d4a954b..e76dbeee979b 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -2875,7 +2875,7 @@ static void binder_transaction(struct binder_proc *proc,
binder_size_t last_fixup_min_off = 0;
struct binder_context *context = proc->context;
int t_debug_id = atomic_inc_return(&binder_last_id);
- struct lsmcontext lsmctx;
+ struct lsmcontext lsmctx = {};
e = binder_transaction_log_add(&binder_transaction_log);
e->debug_id = t_debug_id;
--
James Morris
<jmorris@namei.org>
^ permalink raw reply related
* Re: [PATCH V34 09/29] kexec_file: Restrict at runtime if the kernel is locked down
From: James Morris @ 2019-06-27 18:14 UTC (permalink / raw)
To: Matthew Garrett
Cc: LSM List, Linux Kernel Mailing List, Linux API, Jiri Bohac,
David Howells, kexec
In-Reply-To: <CACdnJusJeCYPKVFHiu6yn+mfqQe5k0RqZhbCUZEkxtXx_shMmw@mail.gmail.com>
On Thu, 27 Jun 2019, Matthew Garrett wrote:
> By that metric, on a secure boot system how do we determine that code
> running in the firmware environment wasn't compromised before it
> launched the initial signed kernel?
Remote attestation tied to a hardware root of trust, before allowing
access to any further resources.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* [RFC PATCH v2 0/3] security/x86/sgx: SGX specific LSM hooks
From: Cedric Xing @ 2019-06-27 18:56 UTC (permalink / raw)
To: linux-sgx, linux-security-module, selinux
Cc: Cedric Xing, casey.schaufler, jmorris, luto, jethro, greg, sds,
jarkko.sakkinen, sean.j.christopherson
In-Reply-To: <20190619222401.14942-1-sean.j.christopherson@intel.com>
This series intends to make the new SGX subsystem to work with the existing LSM
architecture smoothly so that, say, SGX cannot be abused to work around
restrictions set forth by LSM modules/policies.
This patch is based on and could be applied cleanly on top of Jarkko Sakkinen’s
SGX patch series v20 (https://patchwork.kernel.org/cover/10905141/).
For those who haven’t followed closely, the whole discussion started from the
primary question of how to prevent creating an executable enclave page from a
regular memory page that is NOT executable as prohibited by LSM
modules/policies. And that can be translated into 2 relating questions in
practice, i.e. 1) how to determine the allowed initial protections of enclave
pages when they are being loaded and 2) how to determine the allowed
protections of enclave pages at runtime. Those who are familiar with LSM may
notice that, for regular files, #1 is determined by security_mmap_file() while
#2 is covered by security_file_mprotect(). Those 2 hooks however are
insufficient for enclaves due to the distinct composition and lifespan of
enclave pages. Specifically, security_mmap_file() only passes in the file but
is not specific on which portion of the file being mmap()’ed, with the
assumption that all pages of the same file shall have the same set of
allowed/disallowed protections. But that assumption is no longer true for
enclaves for 2 reasons: a) pages of an enclave may be loaded from different
image files with different attributes and b) enclave pages retain contents
across munmap()/mmap(), therefore, say, if a policy prohibits execution of
modified pages, then pages flagged modified have to stay modified across
munmap()/mmap() so that the policy cannot be circumvented by remapping (i.e.
munmap() followed by mmap() on the same range). But the lack of range
information in security_mmap_file()’s arguments simply blocks LSM modules from
tracking enclave pages properly.
A rational solution would always involve tracking the correspondence between
enclave pages and their origin (e.g. files from which they were loaded), which
is similar to tracking regular memory pages and their origin via vm_file of
struct vm_area_struct. But given the longer lifespan of enclave pages (than
VMAs they are mapped into), such correspondence has to be stored in a separate
data structure outside of VMAs. In theory, the correspondence could be stored
either in LSM or in the SGX subsystem. This series has picked the former
because firstly, such information is useful only within LSM so it makes more
sense to keep it as “LSM internal” and secondly, keeping the data structure
inside LSM would allow additional information to be cached in LSM modules
without affecting the rest of the kernel, while lastly, those data structures
would be gone when LSM is disabled hence would not impose any unnecessary
overhead. As you can see in the SELinux implementation of those new hooks
enclosed in this series, options are offered (via kernel command line
parameter) to system administrators between accurate auditing (decisions made
at the time of request) and low memory/performance overhead (decisions
made/cached ahead of time at page instantiation). And that’s one of the
benefits of keeping everything inside LSM.
Those who are familiar with this topic and related discussions may also notice
that, Sean Christopherson has sent out an RFC patch recently to address the
same problem as this series. He adopted the other approach of tracking
page/origin correspondence inside the SGX subsystem. However, to reduce memory
overhead in practice, he cached the FSM (Finite State Machine) instead of
page/origin correspondences. By “FSM”, I mean policy FSM defined as sets of
states and events that may trigger state transitions. Generally speaking, any
LSM module has its own definition of FSM and usually uses attributes attached
to files to argument the FSM, then it advances the FSM as events are observed
and gives out decision based on the current FSM state. Sean’s implementation
attempts to move the FSM into the SGX subsystem, and by caching the arguments
returned by LSM it tries to monitor events and reach the same decisions by
itself. So from architecture perspective, that model has to face tough
challenges in reality, such as how to support multiple LSM modules that employ
different FSMs to govern page protection transitions. Implementation wise, his
model also imposes unwanted restrictions specifically to SGX2, such as:
- Complicated/Restricted UAPI – Enclave loaders are required to provide
“maximal protection” at page load time, but such information is NOT always
available. For example, Graphene containers may run different applications
comprised of different set of executables and/or shared objects. Some of
them may contain self-modifying code (or text relocation) while others
don’t. The generic enclave loader usually doesn’t have such information so
wouldn’t be able to provide it ahead of time.
- Inefficient Auditing – Audit logs are supposed to help system
administrators to determine the set of minimally needed permissions and to
detect abnormal behaviors. But consider the “maximal protection” model, if
“maximal protection” is set to be too permissive, then audit log wouldn’t
be able to detect anomalies; or if “maximal protection” is too restrictive,
then audit log cannot identify the file violating the policy. In either
case the audit log cannot fulfill its purposes.
- Inability to support #PF driven EPC allocation in SGX2 – For those
unfamiliar with SGX2 software flows, an SGX2 enclave requests a page by
issuing EACCEPT on the address that a new page is wanted, and the resulted
#PF is expected to be handled by the kernel by EAUG’ing an EPC page at the
fault address, and then the enclave would be resumed and the faulting
EACCEPT retried, and succeed. The key requirement is to allow mmap()’ing
non-existing enclave pages so that the SGX module/subsystem could respond
to #PFs by EAUG’ing new pages. Sean’s implementation doesn’t allow
mmap()’ing non-existing pages for variety of reasons and therefore blocks
this major SGX2 usage.
History:
- This is version 2 of this patch series, with the following changes per
comments/requests from the community:
+ A new data structure – EMA (Enclave Memory Area) is introduced to track
range/origin correspondences for enclaves. EMAs are maintained by the LSM
framework to be shared among all LSM modules. EMAs are allocated for
enclave files only so will not impose overhead to regular
applications/files.
+ Improved auditing – A new kernel command line option
“lsm.ema.cache_decisions” is introduced, if on, would cause LSM modules
to make/cache decisions at page instantiation (i.e. enclave_load() hook)
instead of at time of request (i.e. file_mprotect() hook), in order to
save memory by NOT keeping enclave source files open. System
administrators are expected to run LSM in permissive mode along with this
option off to figure out the minimal permissions necessary, then turn it
back on in enforcing mode to minimize memory/performance overheads.
+ In the SELinux implementation of the new hooks, FILE__EXECUTE on the file
containing SIGSTRUCT is interpreted as approval for launch, while
FILE__EXECMOD is interpreted as allowing anonymous pages (i.e. pages
EAUG’ed, or EADD’ed from an anonymous source page) to be executable.
Allowed protections for other pages loaded from files are dictated by the
source files’ FILE__EXECUTE/FILE__EXECMOD. This series intentionally
avoids defining new permissions so that user mode tools could continue to
work by treating enclave files the same way as regular executables and/or
shared objects.
- v1 – https://patchwork.kernel.org/cover/10984127/
Cedric Xing (3):
x86/sgx: Add SGX specific LSM hooks
x86/sgx: Call LSM hooks from SGX subsystem/module
x86/sgx: Implement SGX specific hooks in SELinux
arch/x86/kernel/cpu/sgx/driver/ioctl.c | 80 ++++++++-
arch/x86/kernel/cpu/sgx/driver/main.c | 16 +-
include/linux/lsm_ema.h | 171 ++++++++++++++++++
include/linux/lsm_hooks.h | 29 ++++
include/linux/security.h | 23 +++
security/Makefile | 1 +
security/lsm_ema.c | 132 ++++++++++++++
security/security.c | 47 ++++-
security/selinux/hooks.c | 229 ++++++++++++++++++++++++-
security/selinux/include/objsec.h | 24 +++
10 files changed, 732 insertions(+), 20 deletions(-)
create mode 100644 include/linux/lsm_ema.h
create mode 100644 security/lsm_ema.c
--
2.17.1
^ permalink raw reply
* [RFC PATCH v2 1/3] x86/sgx: Add SGX specific LSM hooks
From: Cedric Xing @ 2019-06-27 18:56 UTC (permalink / raw)
To: linux-sgx, linux-security-module, selinux, cedric.xing
Cc: casey.schaufler, jmorris, luto, jethro, greg, sds,
jarkko.sakkinen, sean.j.christopherson
In-Reply-To: <cover.1561588012.git.cedric.xing@intel.com>
SGX enclaves are loaded from pages in regular memory. Given the ability to
create executable pages, the newly added SGX subsystem may present a backdoor
for adversaries to circumvent LSM policies, such as creating an executable
enclave page from a modified regular page that would otherwise not be made
executable as prohibited by LSM. Therefore arises the primary question of
whether an enclave page should be allowed to be created from a given source
page in regular memory.
A related question is whether to grant/deny a mprotect() request on a given
enclave page/range. mprotect() is traditionally covered by
security_file_mprotect() hook, however, enclave pages have a different lifespan
than either MAP_PRIVATE or MAP_SHARED. Particularly, MAP_PRIVATE pages have the
same lifespan as the VMA while MAP_SHARED pages have the same lifespan as the
backing file (on disk), but enclave pages have the lifespan of the enclave’s
file descriptor. For example, enclave pages could be munmap()’ed then mmap()’ed
again without losing contents (like MAP_SHARED), but all enclave pages will be
lost once its file descriptor has been closed (like MAP_PRIVATE). That said,
LSM modules need some new data structure for tracking protections of enclave
pages/ranges so that they can make proper decisions at mmap()/mprotect()
syscalls.
The last question, which is orthogonal to the 2 above, is whether or not to
allow a given enclave to launch/run. Enclave pages are not visible to the rest
of the system, so to some extent offer a better place for malicious software to
hide. Thus, it is sometimes desirable to whitelist/blacklist enclaves by their
measurements, signing public keys, or image files.
To address the questions above, 2 new LSM hooks are added for enclaves.
- security_enclave_load() – This hook allows LSM to decide whether or not to
allow instantiation of a range of enclave pages using the specified VMA. It
is invoked when a range of enclave pages is about to be loaded. It serves 3
purposes: 1) indicate to LSM that the file struct in subject is an enclave;
2) allow LSM to decide whether or not to instantiate those pages and 3)
allow LSM to initialize internal data structures for tracking
origins/protections of those pages.
- security_enclave_init() – This hook allows whitelisting/blacklisting or
performing whatever checks deemed appropriate before an enclave is allowed
to run. An LSM module may opt to use the file backing the SIGSTRUCT as a
proxy to dictate allowed protections for anonymous pages.
mprotect() of enclave pages continue to be governed by
security_file_mprotect(), with the expectation that LSM is able to distinguish
between regular and enclave pages inside the hook. For mmap(), the SGX
subsystem is expected to invoke security_file_mprotect() explicitly to check
protections against the requested protections for existing enclave pages. As
stated earlier, enclave pages have different lifespan than the existing
MAP_PRIVATE and MAP_SHARED pages, so would require a new data structure outside
of VMA to track their protections and/or origins. Enclave Memory Area (or EMA
for short) has been introduced to address the need. EMAs are maintained by the
LSM framework for all LSM modules to share. EMAs will be instantiated for
enclaves only so will not impose memory/performance overheads for regular
applications/files. Please see include/linux/lsm_ema.h and security/lsm_ema.c
for details.
A new setup parameter – lsm.ema.cache_decisions has been introduced to offer
the choice between memory consumption and accuracy of audit logs. Enabling
lsm.ema.cache_decisions causes LSM framework NOT to keep backing files open for
EMAs. While that saves memory, it requires LSM modules to make and cache
decisions ahead of time, and makes it difficult for LSM modules to generate
accurate audit logs. System administrators are expected to run LSM in
permissive mode with lsm.ema.cache_decisions off to determine the minimal
permissions needed, and then turn it back on in enforcing mode for optimal
performance and memory usage. lsm.ema.cache_decisions is on by default and
could be turned off by appending “lsm.ema.cache_decisions=0” or
“lsm.ema.cache_decisions=off” to the kernel command line.
Signed-off-by: Cedric Xing <cedric.xing@intel.com>
---
include/linux/lsm_ema.h | 171 ++++++++++++++++++++++++++++++++++++++
include/linux/lsm_hooks.h | 29 +++++++
include/linux/security.h | 23 +++++
security/Makefile | 1 +
security/lsm_ema.c | 132 +++++++++++++++++++++++++++++
security/security.c | 47 ++++++++++-
6 files changed, 402 insertions(+), 1 deletion(-)
create mode 100644 include/linux/lsm_ema.h
create mode 100644 security/lsm_ema.c
diff --git a/include/linux/lsm_ema.h b/include/linux/lsm_ema.h
new file mode 100644
index 000000000000..a09b8f96da05
--- /dev/null
+++ b/include/linux/lsm_ema.h
@@ -0,0 +1,171 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */
+/**
+ * Enclave Memory Area interface for LSM modules
+ *
+ * Copyright(c) 2016-19 Intel Corporation.
+ */
+
+#ifndef _LSM_EMA_H_
+#define _LSM_EMA_H_
+
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/fs.h>
+#include <linux/file.h>
+
+/**
+ * lsm_ema - LSM Enclave Memory Area structure
+ *
+ * Data structure to track origins of enclave pages
+ *
+ * @link:
+ * Link to adjacent EMAs. EMAs are sorted by their addresses in ascending
+ * order
+ * @start:
+ * Starting address
+ * @end:
+ * Ending address
+ * @source:
+ * File from which this range was loaded from, or NULL if not loaded from
+ * any files
+ */
+struct lsm_ema {
+ struct list_head link;
+ size_t start;
+ size_t end;
+ struct file *source;
+};
+
+#define lsm_ema_data(ema, blob_sizes) \
+ ((char *)((struct lsm_ema *)(ema) + 1) + blob_sizes.lbs_ema_data)
+
+/**
+ * lsm_ema_map - LSM Enclave Memory Map structure
+ *
+ * Container for EMAs of an enclave
+ *
+ * @list:
+ * Head of a list of sorted EMAs
+ * @lock:
+ * Acquire before querying/updateing the list EMAs
+ */
+struct lsm_ema_map {
+ struct list_head list;
+ struct mutex lock;
+};
+
+/**
+ * These are functions to be used by the LSM framework, and must be defined
+ * regardless CONFIG_INTEL_SGX is enabled or not.
+ */
+
+#ifdef CONFIG_INTEL_SGX
+void lsm_ema_global_init(size_t);
+void lsm_free_ema_map(atomic_long_t *);
+#else
+static inline void lsm_ema_global_init(size_t ema_data_size)
+{
+}
+
+static inline void lsm_free_ema_map(atomic_long_t *p)
+{
+}
+#endif
+
+/**
+ * Below are APIs to be used by LSM modules
+ */
+
+struct lsm_ema_map *lsm_init_or_get_ema_map(atomic_long_t *);
+struct lsm_ema *lsm_alloc_ema(void);
+void lsm_free_ema(struct lsm_ema *);
+void lsm_init_ema(struct lsm_ema *, size_t, size_t, struct file *);
+int lsm_merge_ema(struct lsm_ema *, struct lsm_ema_map *);
+struct lsm_ema *lsm_split_ema(struct lsm_ema *, size_t, struct lsm_ema_map *);
+
+static inline struct lsm_ema_map *lsm_get_ema_map(struct file *f)
+{
+ return (void *)atomic_long_read(f->f_security);
+}
+
+static inline int __must_check lsm_lock_ema(struct lsm_ema_map *map)
+{
+ return mutex_lock_interruptible(&map->lock);
+}
+
+static inline void lsm_unlock_ema(struct lsm_ema_map *map)
+{
+ mutex_unlock(&map->lock);
+}
+
+static inline struct lsm_ema *lsm_prev_ema(struct lsm_ema *p,
+ struct lsm_ema_map *map)
+{
+ p = list_prev_entry(p, link);
+ return &p->link == &map->list ? NULL : p;
+}
+
+static inline struct lsm_ema *lsm_next_ema(struct lsm_ema *p,
+ struct lsm_ema_map *map)
+{
+ p = list_next_entry(p, link);
+ return &p->link == &map->list ? NULL : p;
+}
+
+static inline struct lsm_ema *lsm_find_ema(struct lsm_ema_map *map, size_t a)
+{
+ struct lsm_ema *p;
+
+ BUG_ON(!mutex_is_locked(&map->lock));
+
+ list_for_each_entry(p, &map->list, link)
+ if (a < p->end)
+ break;
+ return &p->link == &map->list ? NULL : p;
+}
+
+static inline int lsm_insert_ema(struct lsm_ema_map *map, struct lsm_ema *n)
+{
+ struct lsm_ema *p = lsm_find_ema(map, n->start);
+
+ if (!p)
+ list_add_tail(&n->link, &map->list);
+ else if (n->end <= p->start)
+ list_add_tail(&n->link, &p->link);
+ else
+ return -EEXIST;
+
+ lsm_merge_ema(n, map);
+ if (p)
+ lsm_merge_ema(p, map);
+ return 0;
+}
+
+static inline int lsm_for_each_ema(struct lsm_ema_map *map, size_t start,
+ size_t end, int (*cb)(struct lsm_ema *,
+ void *), void *arg)
+{
+ struct lsm_ema *ema;
+ int rc;
+
+ ema = lsm_find_ema(map, start);
+ while (ema && end > ema->start) {
+ if (start > ema->start)
+ lsm_split_ema(ema, start, map);
+ if (end < ema->end)
+ ema = lsm_split_ema(ema, end, map);
+
+ rc = (*cb)(ema, arg);
+ lsm_merge_ema(ema, map);
+ if (rc)
+ return rc;
+
+ ema = lsm_next_ema(ema, map);
+ }
+
+ if (ema)
+ lsm_merge_ema(ema, map);
+ return 0;
+}
+
+#endif /* _LSM_EMA_H_ */
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 47f58cfb6a19..ade1f9f81e64 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -29,6 +29,8 @@
#include <linux/init.h>
#include <linux/rculist.h>
+struct lsm_ema;
+
/**
* union security_list_options - Linux Security Module hook function list
*
@@ -1446,6 +1448,21 @@
* @bpf_prog_free_security:
* Clean up the security information stored inside bpf prog.
*
+ * @enclave_load:
+ * Decide if a range of pages shall be allowed to be loaded into an
+ * enclave
+ *
+ * @encl points to the file identifying the target enclave
+ * @ema specifies the target range to be loaded
+ * @flags contains protections being requested for the target range
+ * @source points to the VMA containing the source pages to be loaded
+ *
+ * @enclave_init:
+ * Decide if an enclave shall be allowed to launch
+ *
+ * @encl points to the file identifying the target enclave being launched
+ * @sigstruct contains a copy of the SIGSTRUCT in kernel memory
+ * @source points to the VMA backing SIGSTRUCT in user memory
*/
union security_list_options {
int (*binder_set_context_mgr)(struct task_struct *mgr);
@@ -1807,6 +1824,13 @@ union security_list_options {
int (*bpf_prog_alloc_security)(struct bpf_prog_aux *aux);
void (*bpf_prog_free_security)(struct bpf_prog_aux *aux);
#endif /* CONFIG_BPF_SYSCALL */
+
+#ifdef CONFIG_INTEL_SGX
+ int (*enclave_load)(struct file *encl, struct lsm_ema *ema,
+ size_t flags, struct vm_area_struct *source);
+ int (*enclave_init)(struct file *encl, struct sgx_sigstruct *sigstruct,
+ struct vm_area_struct *source);
+#endif
};
struct security_hook_heads {
@@ -2046,6 +2070,10 @@ struct security_hook_heads {
struct hlist_head bpf_prog_alloc_security;
struct hlist_head bpf_prog_free_security;
#endif /* CONFIG_BPF_SYSCALL */
+#ifdef CONFIG_INTEL_SGX
+ struct hlist_head enclave_load;
+ struct hlist_head enclave_init;
+#endif
} __randomize_layout;
/*
@@ -2069,6 +2097,7 @@ struct lsm_blob_sizes {
int lbs_ipc;
int lbs_msg_msg;
int lbs_task;
+ int lbs_ema_data;
};
/*
diff --git a/include/linux/security.h b/include/linux/security.h
index 659071c2e57c..52c200810004 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1829,5 +1829,28 @@ static inline void security_bpf_prog_free(struct bpf_prog_aux *aux)
#endif /* CONFIG_SECURITY */
#endif /* CONFIG_BPF_SYSCALL */
+#ifdef CONFIG_INTEL_SGX
+struct sgx_sigstruct;
+#ifdef CONFIG_SECURITY
+int security_enclave_load(struct file *encl, size_t start, size_t end,
+ size_t flags, struct vm_area_struct *source);
+int security_enclave_init(struct file *encl, struct sgx_sigstruct *sigstruct,
+ struct vm_area_struct *source);
+#else
+static inline int security_enclave_load(struct file *encl, size_t start,
+ size_t end, struct vm_area_struct *src)
+{
+ return 0;
+}
+
+static inline int security_enclave_init(struct file *encl,
+ struct sgx_sigstruct *sigstruct,
+ struct vm_area_struct *src)
+{
+ return 0;
+}
+#endif /* CONFIG_SECURITY */
+#endif /* CONFIG_INTEL_SGX */
+
#endif /* ! __LINUX_SECURITY_H */
diff --git a/security/Makefile b/security/Makefile
index c598b904938f..1bab8f1344b6 100644
--- a/security/Makefile
+++ b/security/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_SECURITY_YAMA) += yama/
obj-$(CONFIG_SECURITY_LOADPIN) += loadpin/
obj-$(CONFIG_SECURITY_SAFESETID) += safesetid/
obj-$(CONFIG_CGROUP_DEVICE) += device_cgroup.o
+obj-$(CONFIG_INTEL_SGX) += lsm_ema.o
# Object integrity file lists
subdir-$(CONFIG_INTEGRITY) += integrity
diff --git a/security/lsm_ema.c b/security/lsm_ema.c
new file mode 100644
index 000000000000..68fae0724d37
--- /dev/null
+++ b/security/lsm_ema.c
@@ -0,0 +1,132 @@
+// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+// Copyright(c) 2016-18 Intel Corporation.
+
+#include <linux/lsm_ema.h>
+#include <linux/slab.h>
+
+static struct kmem_cache *lsm_ema_cache;
+static size_t lsm_ema_data_size;
+static int lsm_ema_cache_decisions = 1;
+
+void lsm_ema_global_init(size_t ema_size)
+{
+ BUG_ON(lsm_ema_data_size > 0);
+
+ lsm_ema_data_size = ema_size;
+
+ ema_size += sizeof(struct lsm_ema);
+ ema_size = max(ema_size, sizeof(struct lsm_ema_map));
+ lsm_ema_cache = kmem_cache_create("lsm_ema_cache", ema_size,
+ __alignof__(struct lsm_ema),
+ SLAB_PANIC, NULL);
+
+}
+
+struct lsm_ema_map *lsm_init_or_get_ema_map(atomic_long_t *p)
+{
+ struct lsm_ema_map *map;
+
+ map = (typeof(map))atomic_long_read(p);
+ if (!map) {
+ long n;
+
+ map = (typeof(map))lsm_alloc_ema();
+ if (!map)
+ return NULL;
+
+ INIT_LIST_HEAD(&map->list);
+ mutex_init(&map->lock);
+
+ n = atomic_long_cmpxchg(p, 0, (long)map);
+ if (n) {
+ atomic_long_t a;
+ atomic_long_set(&a, (long)map);
+ map = (typeof(map))n;
+ lsm_free_ema_map(&a);
+ }
+ }
+ return map;
+}
+
+void lsm_free_ema_map(atomic_long_t *p)
+{
+ struct lsm_ema_map *map;
+ struct lsm_ema *ema, *n;
+
+ map = (typeof(map))atomic_long_read(p);
+ if (!map)
+ return;
+
+ BUG_ON(mutex_is_locked(&map->lock));
+
+ list_for_each_entry_safe(ema, n, &map->list, link)
+ lsm_free_ema(ema);
+ kmem_cache_free(lsm_ema_cache, map);
+}
+
+struct lsm_ema *lsm_alloc_ema(void)
+{
+ return kmem_cache_zalloc(lsm_ema_cache, GFP_KERNEL);
+}
+
+void lsm_free_ema(struct lsm_ema *ema)
+{
+ list_del(&ema->link);
+ if (ema->source)
+ fput(ema->source);
+ kmem_cache_free(lsm_ema_cache, ema);
+}
+
+void lsm_init_ema(struct lsm_ema *ema, size_t start, size_t end,
+ struct file *source)
+{
+ INIT_LIST_HEAD(&ema->link);
+ ema->start = start;
+ ema->end = end;
+ if (!lsm_ema_cache_decisions && source)
+ ema->source = get_file(source);
+}
+
+int lsm_merge_ema(struct lsm_ema *p, struct lsm_ema_map *map)
+{
+ struct lsm_ema *prev = list_prev_entry(p, link);
+
+ BUG_ON(!mutex_is_locked(&map->lock));
+
+ if (&prev->link == &map->list || prev->end != p->start ||
+ prev->source != p->source ||
+ memcmp(prev + 1, p + 1, lsm_ema_data_size))
+ return 0;
+
+ p->start = prev->start;
+ fput(prev->source);
+ lsm_free_ema(prev);
+ return 1;
+}
+
+struct lsm_ema *lsm_split_ema(struct lsm_ema *p, size_t at,
+ struct lsm_ema_map *map)
+{
+ struct lsm_ema *n;
+
+ BUG_ON(!mutex_is_locked(&map->lock));
+
+ if (at <= p->start || at >= p->end)
+ return p;
+
+ n = lsm_alloc_ema();
+ if (likely(n)) {
+ lsm_init_ema(n, p->start, at, p->source);
+ memcpy(n + 1, p + 1, lsm_ema_data_size);
+ p->start = at;
+ list_add_tail(&n->link, &p->link);
+ }
+ return n;
+}
+
+static int __init set_ema_cache_decisions(char *str)
+{
+ lsm_ema_cache_decisions = (strcmp(str, "0") && strcmp(str, "off"));
+ return 1;
+}
+__setup("lsm.ema.cache_decisions=", set_ema_cache_decisions);
diff --git a/security/security.c b/security/security.c
index f493db0bf62a..d50883f18be2 100644
--- a/security/security.c
+++ b/security/security.c
@@ -17,6 +17,7 @@
#include <linux/init.h>
#include <linux/kernel.h>
#include <linux/lsm_hooks.h>
+#include <linux/lsm_ema.h>
#include <linux/integrity.h>
#include <linux/ima.h>
#include <linux/evm.h>
@@ -41,7 +42,9 @@ static struct kmem_cache *lsm_file_cache;
static struct kmem_cache *lsm_inode_cache;
char *lsm_names;
-static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init;
+static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init = {
+ .lbs_file = sizeof(atomic_long_t) * IS_ENABLED(CONFIG_INTEL_SGX),
+};
/* Boot-time LSM user choice */
static __initdata const char *chosen_lsm_order;
@@ -169,6 +172,7 @@ static void __init lsm_set_blob_sizes(struct lsm_blob_sizes *needed)
lsm_set_blob_size(&needed->lbs_ipc, &blob_sizes.lbs_ipc);
lsm_set_blob_size(&needed->lbs_msg_msg, &blob_sizes.lbs_msg_msg);
lsm_set_blob_size(&needed->lbs_task, &blob_sizes.lbs_task);
+ lsm_set_blob_size(&needed->lbs_ema_data, &blob_sizes.lbs_ema_data);
}
/* Prepare LSM for initialization. */
@@ -314,6 +318,7 @@ static void __init ordered_lsm_init(void)
lsm_inode_cache = kmem_cache_create("lsm_inode_cache",
blob_sizes.lbs_inode, 0,
SLAB_PANIC, NULL);
+ lsm_ema_global_init(blob_sizes.lbs_ema_data);
lsm_early_cred((struct cred *) current->cred);
lsm_early_task(current);
@@ -1357,6 +1362,7 @@ void security_file_free(struct file *file)
blob = file->f_security;
if (blob) {
file->f_security = NULL;
+ lsm_free_ema_map(blob);
kmem_cache_free(lsm_file_cache, blob);
}
}
@@ -1420,6 +1426,7 @@ int security_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot,
{
return call_int_hook(file_mprotect, 0, vma, reqprot, prot);
}
+EXPORT_SYMBOL(security_file_mprotect);
int security_file_lock(struct file *file, unsigned int cmd)
{
@@ -2355,3 +2362,41 @@ void security_bpf_prog_free(struct bpf_prog_aux *aux)
call_void_hook(bpf_prog_free_security, aux);
}
#endif /* CONFIG_BPF_SYSCALL */
+
+#ifdef CONFIG_INTEL_SGX
+int security_enclave_load(struct file *encl, size_t start, size_t end,
+ size_t flags, struct vm_area_struct *src)
+{
+ struct lsm_ema_map *map;
+ struct lsm_ema *ema;
+ int rc;
+
+ map = lsm_init_or_get_ema_map(encl->f_security);
+ if (unlikely(!map))
+ return -ENOMEM;
+
+ ema = lsm_alloc_ema();
+ if (unlikely(!ema))
+ return -ENOMEM;
+
+ lsm_init_ema(ema, start, end, src->vm_file);
+ rc = call_int_hook(enclave_load, 0, encl, ema, flags, src);
+ if (!rc)
+ rc = lsm_lock_ema(map);
+ if (!rc) {
+ rc = lsm_insert_ema(map, ema);
+ lsm_unlock_ema(map);
+ }
+ if (rc)
+ lsm_free_ema(ema);
+ return rc;
+}
+EXPORT_SYMBOL(security_enclave_load);
+
+int security_enclave_init(struct file *encl, struct sgx_sigstruct *sigstruct,
+ struct vm_area_struct *src)
+{
+ return call_int_hook(enclave_init, 0, encl, sigstruct, src);
+}
+EXPORT_SYMBOL(security_enclave_init);
+#endif /* CONFIG_INTEL_SGX */
--
2.17.1
^ permalink raw reply related
* [RFC PATCH v2 2/3] x86/sgx: Call LSM hooks from SGX subsystem/module
From: Cedric Xing @ 2019-06-27 18:56 UTC (permalink / raw)
To: linux-sgx, linux-security-module, selinux, cedric.xing
Cc: casey.schaufler, jmorris, luto, jethro, greg, sds,
jarkko.sakkinen, sean.j.christopherson
In-Reply-To: <cover.1561588012.git.cedric.xing@intel.com>
It’s straightforward to call new LSM hooks from the SGX subsystem/module. There
are three places where LSM hooks are invoked.
1) sgx_mmap() invokes security_file_mprotect() to validate requested
protection. It is necessary because security_mmap_file() invoked by mmap()
syscall only validates protections against /dev/sgx/enclave file, but not
against those files from which the pages were loaded from.
2) security_enclave_load() is invoked upon loading of every enclave page by
the EADD ioctl. Please note that if pages are EADD’ed in batch, the SGX
subsystem/module is responsible for dividing pages in trunks so that each
trunk is loaded from a single VMA.
3) security_enclave_init() is invoked before initializing (EINIT) every
enclave.
Signed-off-by: Cedric Xing <cedric.xing@intel.com>
---
arch/x86/kernel/cpu/sgx/driver/ioctl.c | 80 +++++++++++++++++++++++---
arch/x86/kernel/cpu/sgx/driver/main.c | 16 +++++-
2 files changed, 85 insertions(+), 11 deletions(-)
diff --git a/arch/x86/kernel/cpu/sgx/driver/ioctl.c b/arch/x86/kernel/cpu/sgx/driver/ioctl.c
index b186fb7b48d5..4f5abf9819a7 100644
--- a/arch/x86/kernel/cpu/sgx/driver/ioctl.c
+++ b/arch/x86/kernel/cpu/sgx/driver/ioctl.c
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
// Copyright(c) 2016-19 Intel Corporation.
-#include <asm/mman.h>
+#include <linux/mman.h>
#include <linux/delay.h>
#include <linux/file.h>
#include <linux/hashtable.h>
@@ -11,6 +11,7 @@
#include <linux/shmem_fs.h>
#include <linux/slab.h>
#include <linux/suspend.h>
+#include <linux/security.h>
#include "driver.h"
struct sgx_add_page_req {
@@ -575,6 +576,46 @@ static int sgx_encl_add_page(struct sgx_encl *encl, unsigned long addr,
return ret;
}
+static int sgx_encl_prepare_page(struct file *filp, unsigned long dst,
+ unsigned long src, void *buf)
+{
+ struct vm_area_struct *vma;
+ unsigned long prot;
+ int rc;
+
+ if (dst & ~PAGE_SIZE)
+ return -EINVAL;
+
+ rc = down_read_killable(¤t->mm->mmap_sem);
+ if (rc)
+ return rc;
+
+ vma = find_vma(current->mm, dst);
+ if (vma && dst >= vma->vm_start)
+ prot = _calc_vm_trans(vma->vm_flags, VM_READ, PROT_READ) |
+ _calc_vm_trans(vma->vm_flags, VM_WRITE, PROT_WRITE) |
+ _calc_vm_trans(vma->vm_flags, VM_EXEC, PROT_EXEC);
+ else
+ prot = 0;
+
+ vma = find_vma(current->mm, src);
+ if (!vma || src < vma->vm_start || src + PAGE_SIZE > vma->vm_end)
+ rc = -EFAULT;
+
+ if (!rc && !(vma->vm_flags & VM_MAYEXEC))
+ rc = -EACCES;
+
+ if (!rc && copy_from_user(buf, (void __user *)src, PAGE_SIZE))
+ rc = -EFAULT;
+
+ if (!rc)
+ rc = security_enclave_load(filp, dst, PAGE_SIZE, prot, vma);
+
+ up_read(¤t->mm->mmap_sem);
+
+ return rc;
+}
+
/**
* sgx_ioc_enclave_add_page - handler for %SGX_IOC_ENCLAVE_ADD_PAGE
*
@@ -613,10 +654,9 @@ static long sgx_ioc_enclave_add_page(struct file *filep, unsigned int cmd,
data = kmap(data_page);
- if (copy_from_user((void *)data, (void __user *)addp->src, PAGE_SIZE)) {
- ret = -EFAULT;
+ ret = sgx_encl_prepare_page(filep, addp->addr, addp->src, data);
+ if (ret)
goto out;
- }
ret = sgx_encl_add_page(encl, addp->addr, data, &secinfo, addp->mrmask);
if (ret)
@@ -718,6 +758,31 @@ static int sgx_encl_init(struct sgx_encl *encl, struct sgx_sigstruct *sigstruct,
return ret;
}
+static int sgx_encl_prepare_sigstruct(struct file *filp, unsigned long src,
+ struct sgx_sigstruct *ss)
+{
+ struct vm_area_struct *vma;
+ int rc;
+
+ rc = down_read_killable(¤t->mm->mmap_sem);
+ if (rc)
+ return rc;
+
+ vma = find_vma(current->mm, src);
+ if (!vma || src < vma->vm_start || src + sizeof(*ss) > vma->vm_end)
+ rc = -EFAULT;
+
+ if (!rc && copy_from_user(ss, (void __user *)src, sizeof(*ss)))
+ rc = -EFAULT;
+
+ if (!rc)
+ rc = security_enclave_init(filp, ss, vma);
+
+ up_read(¤t->mm->mmap_sem);
+
+ return rc;
+}
+
/**
* sgx_ioc_enclave_init - handler for %SGX_IOC_ENCLAVE_INIT
*
@@ -753,12 +818,9 @@ static long sgx_ioc_enclave_init(struct file *filep, unsigned int cmd,
((unsigned long)sigstruct + PAGE_SIZE / 2);
memset(einittoken, 0, sizeof(*einittoken));
- if (copy_from_user(sigstruct, (void __user *)initp->sigstruct,
- sizeof(*sigstruct))) {
- ret = -EFAULT;
+ ret = sgx_encl_prepare_sigstruct(filep, initp->sigstruct, sigstruct);
+ if (ret)
goto out;
- }
-
ret = sgx_encl_init(encl, sigstruct, einittoken);
diff --git a/arch/x86/kernel/cpu/sgx/driver/main.c b/arch/x86/kernel/cpu/sgx/driver/main.c
index afe844aa81d6..95fe18c37b84 100644
--- a/arch/x86/kernel/cpu/sgx/driver/main.c
+++ b/arch/x86/kernel/cpu/sgx/driver/main.c
@@ -63,14 +63,26 @@ static long sgx_compat_ioctl(struct file *filep, unsigned int cmd,
static int sgx_mmap(struct file *file, struct vm_area_struct *vma)
{
struct sgx_encl *encl = file->private_data;
+ unsigned long prot;
+ int rc;
vma->vm_ops = &sgx_vm_ops;
vma->vm_flags |= VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP | VM_IO;
vma->vm_private_data = encl;
- kref_get(&encl->refcount);
+ prot = vma->vm_flags & (VM_READ | VM_WRITE | VM_EXEC);
+ vma->vm_flags &= ~prot;
- return 0;
+ prot = _calc_vm_trans(prot, VM_READ, PROT_READ) |
+ _calc_vm_trans(prot, VM_WRITE, PROT_WRITE) |
+ _calc_vm_trans(prot, VM_EXEC, PROT_EXEC);
+ rc = security_file_mprotect(vma, prot, prot);
+ if (!rc) {
+ vma->vm_flags |= calc_vm_prot_bits(prot, 0);
+ kref_get(&encl->refcount);
+ }
+
+ return rc;
}
static unsigned long sgx_get_unmapped_area(struct file *file,
--
2.17.1
^ permalink raw reply related
* [RFC PATCH v2 3/3] x86/sgx: Implement SGX specific hooks in SELinux
From: Cedric Xing @ 2019-06-27 18:56 UTC (permalink / raw)
To: linux-sgx, linux-security-module, selinux, cedric.xing
Cc: casey.schaufler, jmorris, luto, jethro, greg, sds,
jarkko.sakkinen, sean.j.christopherson
In-Reply-To: <cover.1561588012.git.cedric.xing@intel.com>
This patch governs enclave page protections in a similar way to how current
SELinux governs protections for regular memory pages. In summary:
- All pages are allowed PROT_READ/PROT_WRITE upon request.
- For pages that are EADD’ed, PROT_EXEC will be granted initially if
PROT_EXEC could also be granted to the VMA containing the source pages.
Afterwards, PROT_EXEC will be removed once PROT_WRITE is requested/granted,
and could be granted again if the backing file has EXECMOD or the calling
process has PROCMEM. For anonymous pages, backing file is considered to be
the file containing SIGSTRUCT.
- For pages that are EAUG’ed, they are considered modified initially so
PROT_EXEC will not be granted unless the file containing SIGSTRUCT has
EXECMOD, or the calling process has EXECMEM.
Besides, launch control is implemented as EXECUTE permission on the SIGSTRUCT
file. That is,
- SIGSTRUCT file has EXECUTE – Enclave is allowed to launch. But this is
granted only if the enclosing VMA has the same content as the disk file
(i.e. vma->anon_vma == NULL).
- SIGSTRUCT file has EXECMOD – All anonymous enclave pages are allowed
PROT_EXEC.
In all cases, simultaneous WX requires EXECMEM on the calling process.
Implementation wise, 3 bits are associated with every EMA by SELinux.
- sourced – Set if EMA is loaded from a file, cleared otherwise.
- execute – Set if EMA is potentially executable, cleared when EMA has once
been mapped writable, as result of mmap()/mprotect() syscalls. A page is
executable if this bit is set AND its backing file or the file containing
SIGSTRUCT (for anonymous pages) has EXECUTE. This bit will be cleared upon
PROT_WRITE granted to the EMA.
- execmod – Set if the backing file or the file containing SIGSTRUCT (for
anonymous pages) has EXECMOD. A page is executable if this bit is set.
All those 3 bits are initialized at selinux_enclave_load() and checked in
selinux_file_mprotect(). SGX subsystem is expected to invoke
security_file_mprotect() upon mmap() to not bypass the check. mmap() shall be
treated as mprotect() from PROT_NONE to the requested protection.
selinux_enclave_init() determines if an enclave is allowed to launch, using the
criteria described earlier. This implementation does NOT accept SIGSTRUCT in
anonymous memory. The backing file is also cached in struct
file_security_struct and will serve as the base for decisions for anonymous
pages.
There are NO new process/file permissions introduced in this patch. The
intention here is to ensure existing SELinux tools will work with enclaves
seamlessly by treating them the same way as regular shared objects.
Signed-off-by: Cedric Xing <cedric.xing@intel.com>
---
security/selinux/hooks.c | 229 ++++++++++++++++++++++++++++--
security/selinux/include/objsec.h | 24 ++++
2 files changed, 245 insertions(+), 8 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 94de51628fdc..cea4db780eb8 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -1663,10 +1663,9 @@ static int cred_has_capability(const struct cred *cred,
/* Check whether a task has a particular permission to an inode.
The 'adp' parameter is optional and allows other audit
data to be passed (e.g. the dentry). */
-static int inode_has_perm(const struct cred *cred,
- struct inode *inode,
- u32 perms,
- struct common_audit_data *adp)
+static inline int inode_has_perm_audit(int audit, const struct cred *cred,
+ struct inode *inode, u32 perms,
+ struct common_audit_data *adp)
{
struct inode_security_struct *isec;
u32 sid;
@@ -1679,8 +1678,22 @@ static int inode_has_perm(const struct cred *cred,
sid = cred_sid(cred);
isec = selinux_inode(inode);
- return avc_has_perm(&selinux_state,
- sid, isec->sid, isec->sclass, perms, adp);
+ if (audit)
+ return avc_has_perm(&selinux_state, sid, isec->sid,
+ isec->sclass, perms, adp);
+ else {
+ struct av_decision avd;
+ return avc_has_perm_noaudit(&selinux_state, sid, isec->sid,
+ isec->sclass, perms, 0, &avd);
+ }
+}
+
+static int inode_has_perm(const struct cred *cred,
+ struct inode *inode,
+ u32 perms,
+ struct common_audit_data *adp)
+{
+ return inode_has_perm_audit(1, cred, inode, perms, adp);
}
/* Same as inode_has_perm, but pass explicit audit data containing
@@ -3499,6 +3512,13 @@ static int selinux_file_alloc_security(struct file *file)
return file_alloc_security(file);
}
+static void selinux_file_free_security(struct file *file)
+{
+ long f = atomic_long_read(&selinux_file(file)->enclave_proxy_file);
+ if (f)
+ fput((struct file *)f);
+}
+
/*
* Check whether a task has the ioctl permission and cmd
* operation to an inode.
@@ -3666,19 +3686,23 @@ static int selinux_mmap_file(struct file *file, unsigned long reqprot,
(flags & MAP_TYPE) == MAP_SHARED);
}
+#ifdef CONFIG_INTEL_SGX
+static int enclave_mprotect(struct vm_area_struct *, size_t);
+#endif
+
static int selinux_file_mprotect(struct vm_area_struct *vma,
unsigned long reqprot,
unsigned long prot)
{
const struct cred *cred = current_cred();
u32 sid = cred_sid(cred);
+ int rc = 0;
if (selinux_state.checkreqprot)
prot = reqprot;
if (default_noexec &&
(prot & PROT_EXEC) && !(vma->vm_flags & VM_EXEC)) {
- int rc = 0;
if (vma->vm_start >= vma->vm_mm->start_brk &&
vma->vm_end <= vma->vm_mm->brk) {
rc = avc_has_perm(&selinux_state,
@@ -3705,7 +3729,12 @@ static int selinux_file_mprotect(struct vm_area_struct *vma,
return rc;
}
- return file_map_prot_check(vma->vm_file, prot, vma->vm_flags&VM_SHARED);
+ rc = file_map_prot_check(vma->vm_file, prot, vma->vm_flags&VM_SHARED);
+#ifdef CONFIG_INTEL_SGX
+ if (!rc)
+ rc = enclave_mprotect(vma, prot);
+#endif
+ return rc;
}
static int selinux_file_lock(struct file *file, unsigned int cmd)
@@ -6740,12 +6769,190 @@ static void selinux_bpf_prog_free(struct bpf_prog_aux *aux)
}
#endif
+#ifdef CONFIG_INTEL_SGX
+struct ema__mprot_cb_params {
+ struct file *encl;
+ size_t curprot;
+ size_t reqprot;
+};
+
+static inline struct file *ema__get_source(struct lsm_ema *ema,
+ struct file *encl)
+{
+ if (!selinux_ema(ema)->sourced) {
+ struct file_security_struct *fsec = selinux_file(encl);
+ return (void *)atomic_long_read(&fsec->enclave_proxy_file);
+ }
+
+ return ema->source;
+}
+
+static int ema__chk_X_cb(struct lsm_ema *ema, void *a)
+{
+ const struct ema__mprot_cb_params *parm = a;
+ struct ema_security_struct *esec = selinux_ema(ema);
+ struct file *src;
+ int rc;
+
+ if (esec->execmod)
+ /* EXECMOD grants X on all cases */
+ return 0;
+
+ src = ema__get_source(ema, parm->encl);
+ if (src) {
+ if (esec->execute)
+ /* Unmodified range requires FILE__EXECUTE */
+ rc = file_has_perm(current_cred(), src,
+ FILE__EXECUTE);
+ else {
+ /* Modified range requires FILE__EXECMOD */
+ rc = file_has_perm(current_cred(), src,
+ FILE__EXECUTE | FILE__EXECMOD);
+ /* Cache FILE__EXECMOD to avoid checking it again */
+ esec->execmod = !rc;
+ }
+ } else
+ rc = esec->execute ? 0 : -EACCES;
+ return rc;
+}
+
+static int ema__clr_X_cb(struct lsm_ema *ema, void *a)
+{
+ selinux_ema(ema)->execute = 0;
+ return 0;
+}
+
+static int enclave_mprotect(struct vm_area_struct *vma, size_t prot)
+{
+ struct lsm_ema_map *map;
+ int rc;
+
+ if (!vma->vm_file)
+ return 0;
+
+ map = lsm_get_ema_map(vma->vm_file);
+ if (!map)
+ /* Not an enclave */
+ return 0;
+
+ if ((prot & VM_WRITE) && (prot && VM_EXEC)) {
+ /* EXECMEM is necessary, and will be checked later */
+ rc = -1;
+ } else {
+ struct ema__mprot_cb_params parm;
+
+ parm.encl = vma->vm_file;
+ parm.curprot = vma->vm_flags & (VM_READ | VM_WRITE | VM_EXEC);
+ parm.reqprot = calc_vm_prot_bits(prot, 0);
+
+ rc = lsm_lock_ema(map);
+ if (!rc)
+ return rc;
+
+ /* Checks are necessary only if X is being requested */
+ if (prot & VM_EXEC)
+ rc = lsm_for_each_ema(map, vma->vm_start, vma->vm_end,
+ ema__chk_X_cb, &parm);
+ /* Clear X if W is granted */
+ if (!rc && (prot & VM_WRITE))
+ rc = lsm_for_each_ema(map, vma->vm_start, vma->vm_end,
+ ema__clr_X_cb, &parm);
+ lsm_unlock_ema(map);
+ }
+
+ /* EXECMEM is the last resort if X is being requested */
+ if (rc && (prot & VM_EXEC)) {
+ /* No need to update selinux_ema(ema)->execute here because it
+ * doesn't matter anyway when EXECMEM is present
+ */
+ rc = avc_has_perm(&selinux_state, current_sid(), current_sid(),
+ SECCLASS_PROCESS, PROCESS__EXECMEM, NULL);
+ }
+ return rc;
+}
+
+static int selinux_enclave_load(struct file *encl, struct lsm_ema *ema,
+ size_t flags, struct vm_area_struct *src)
+{
+ size_t prot = flags & (PROT_READ | PROT_WRITE | PROT_EXEC);
+ struct ema_security_struct *esec;
+ const struct cred *cred = current_cred();
+ u32 sid = cred_sid(cred);
+ int rc;
+
+ /* check if @prot could be granted */
+ rc = 0;
+ if (src) {
+ /* EADD */
+ if (calc_vm_prot_bits(prot, 0) & ~src->vm_flags)
+ rc = selinux_file_mprotect(src, prot, prot);
+ } else if (prot & PROT_EXEC) {
+ /* EAUG implies RW, so RWX here requires EXECMEM */
+ rc = avc_has_perm(&selinux_state, sid, sid,
+ SECCLASS_PROCESS, PROCESS__EXECMEM, NULL);
+ }
+ if (rc)
+ return rc;
+
+ /* Initialize ema_security_struct now that @prot has been approved */
+ esec = selinux_ema(ema);
+ /* Is @src backed by a file? */
+ if (src && src->vm_file)
+ esec->sourced = 1;
+ /* Is @src mapped shared, or mapped privately and not modified? */
+ if ((esec->sourced && !src->anon_vma) || (prot & PROT_EXEC))
+ esec->execute = 1;
+ /* If the backing file is NOT kept opened, cache FILE__EXECUTE now! No
+ * audit log will be generated */
+ if (esec->execute && esec->sourced && !ema->source &&
+ inode_has_perm_audit(0, cred, file_inode(src->vm_file),
+ FILE__EXECUTE, NULL))
+ esec->execute = 0;
+ /* If the backing file is NOT kept opened, cache FILE__EXECMOD now! No
+ * audit log will be generated */
+ if (esec->sourced && !ema->source &&
+ !inode_has_perm_audit(0, cred, file_inode(src->vm_file),
+ FILE__EXECUTE | FILE__EXECMOD, NULL))
+ esec->execmod = 1;
+
+ return 0;
+}
+
+static int selinux_enclave_init(struct file *encl,
+ struct sgx_sigstruct *sigstruct,
+ struct vm_area_struct *src)
+{
+ struct file_security_struct *fsec = selinux_file(encl);
+ int rc;
+
+ /* Is @src mapped shared, or mapped privately and not modified? */
+ if (!src->vm_file || src->anon_vma)
+ return -EACCES;
+
+ /* FILE__EXECUTE grants enclaves permission to launch */
+ rc = file_has_perm(current_cred(), src->vm_file, FILE__EXECUTE);
+ if (rc)
+ return rc;
+
+ /* SIGSTRUCT file is also used to determine permissions for pages not
+ * backed by any files */
+ if (atomic_long_cmpxchg(&fsec->enclave_proxy_file, 0,
+ (long)src->vm_file))
+ return -EEXIST;
+
+ get_file(src->vm_file);
+ return 0;
+}
+#endif
+
struct lsm_blob_sizes selinux_blob_sizes __lsm_ro_after_init = {
.lbs_cred = sizeof(struct task_security_struct),
.lbs_file = sizeof(struct file_security_struct),
.lbs_inode = sizeof(struct inode_security_struct),
.lbs_ipc = sizeof(struct ipc_security_struct),
.lbs_msg_msg = sizeof(struct msg_security_struct),
+ .lbs_ema_data = sizeof(struct ema_security_struct) *
+ IS_ENABLED(CONFIG_INTEL_SGX),
};
static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
@@ -6822,6 +7029,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(file_permission, selinux_file_permission),
LSM_HOOK_INIT(file_alloc_security, selinux_file_alloc_security),
+ LSM_HOOK_INIT(file_free_security, selinux_file_free_security),
LSM_HOOK_INIT(file_ioctl, selinux_file_ioctl),
LSM_HOOK_INIT(mmap_file, selinux_mmap_file),
LSM_HOOK_INIT(mmap_addr, selinux_mmap_addr),
@@ -6982,6 +7190,11 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(bpf_map_free_security, selinux_bpf_map_free),
LSM_HOOK_INIT(bpf_prog_free_security, selinux_bpf_prog_free),
#endif
+
+#ifdef CONFIG_INTEL_SGX
+ LSM_HOOK_INIT(enclave_load, selinux_enclave_load),
+ LSM_HOOK_INIT(enclave_init, selinux_enclave_init),
+#endif
};
static __init int selinux_init(void)
diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
index 91c5395dd20c..e58324997e8b 100644
--- a/security/selinux/include/objsec.h
+++ b/security/selinux/include/objsec.h
@@ -23,6 +23,7 @@
#include <linux/in.h>
#include <linux/spinlock.h>
#include <linux/lsm_hooks.h>
+#include <linux/lsm_ema.h>
#include <linux/msg.h>
#include <net/net_namespace.h>
#include "flask.h"
@@ -68,6 +69,7 @@ struct file_security_struct {
u32 fown_sid; /* SID of file owner (for SIGIO) */
u32 isid; /* SID of inode at the time of file open */
u32 pseqno; /* Policy seqno at the time of file open */
+ atomic_long_t enclave_proxy_file;
};
struct superblock_security_struct {
@@ -154,6 +156,23 @@ struct bpf_security_struct {
u32 sid; /*SID of bpf obj creater*/
};
+struct ema_security_struct {
+ /* (@execute && FILE__EXECUTE) grants X.
+ * FILE__EXECUTE is determined at mprotect() but if backing file is NOT
+ * kept open, FILE__EXECUTE will be determined at enclave_load() hook
+ */
+ int execute:1;
+ /* (@execmod || FILE__EXECMOD) grants W->X.
+ * FILE__EXECMOD is determined at mprotect() but if backing file is NOT
+ * kept open, FILE__EXECMOD will be determined at enclave_load() hook
+ */
+ int execmod:1;
+ /* @sourced is set if an enclave range is loaded (EADD'ed) from a file,
+ * cleared otherwise (i.e. EAUG'ed or EADD'ed from anonymous memory
+ */
+ int sourced:1;
+};
+
extern struct lsm_blob_sizes selinux_blob_sizes;
static inline struct task_security_struct *selinux_cred(const struct cred *cred)
{
@@ -185,4 +204,9 @@ static inline struct ipc_security_struct *selinux_ipc(
return ipc->security + selinux_blob_sizes.lbs_ipc;
}
+static inline struct ema_security_struct *selinux_ema(struct lsm_ema *ema)
+{
+ return (void *)lsm_ema_data(ema, selinux_blob_sizes);
+}
+
#endif /* _SELINUX_OBJSEC_H_ */
--
2.17.1
^ permalink raw reply related
* RE: LSM module for SGX?
From: Xing, Cedric @ 2019-06-27 19:20 UTC (permalink / raw)
To: Stephen Smalley, Jarkko Sakkinen,
linux-security-module@vger.kernel.org, linux-sgx@vger.kernel.org,
x86@kernel.org
Cc: Schaufler, Casey, jmorris@namei.org, luto@kernel.org,
Christopherson, Sean J
In-Reply-To: <496ea018-2655-a438-bc6b-80330c36cd11@tycho.nsa.gov>
> From: linux-sgx-owner@vger.kernel.org [mailto:linux-sgx-
> owner@vger.kernel.org] On Behalf Of Stephen Smalley
> Sent: Thursday, June 27, 2019 6:42 AM
>
> On 6/27/19 8:56 AM, Jarkko Sakkinen wrote:
> > Looking at the SGX-LSM discussions I haven't seen even a single email
> > that would have any conclusions that the new hooks are the only
> possible
> > route to limit the privileges to use SGX.
> >
> > An obvious alternative to consider might be to have a small-scale LSM
> > that you could stack. AFAIK Casey's LSM stacking patch set has not yet
> > landed but I also remember that with some constraints you can still do
> > it. Casey explained these constraints to me few years ago but I can't
> > recall them anymore :-)
> >
> > One example of this is Yama, which limits the use of ptrace(). You can
> > enable it together with any of the "big" LSMs in the kernel.
> >
> > A major benefit in this approach would that it is non-intrusive when
> it
> > comes to other architectures than x86. New hooks are not only
> > maintenance burden for those who care about SGX but also for those who
> > have to deal with LSMs.
>
> Regardless of whether or not you or anyone else creates such a
> small-scale LSM, we would still want to be able to control the loading
> of enclaves and the creation of executable enclave mappings via SELinux
> policy, so the hooks would be necessary anyway.
Just want to add that, no matter it is incorporated into a big or separated into a small LSM module, hooks are always necessary. The difference is just which LSM module registers for those hooks.
^ permalink raw reply
* RE: [RFC PATCH v4 09/12] LSM: x86/sgx: Introduce ->enclave_load() hook for Intel SGX
From: Xing, Cedric @ 2019-06-27 19:38 UTC (permalink / raw)
To: Stephen Smalley, Christopherson, Sean J, Jarkko Sakkinen
Cc: linux-sgx@vger.kernel.org, linux-security-module@vger.kernel.org,
selinux@vger.kernel.org, Roberts, William C, Schaufler, Casey,
James Morris, Hansen, Dave, Andy Lutomirski, Jethro Beekman,
Dr . Greg Wettstein
In-Reply-To: <a494b094-cde7-b53d-3903-81ccc907b392@tycho.nsa.gov>
> From: linux-sgx-owner@vger.kernel.org [mailto:linux-sgx-
> owner@vger.kernel.org] On Behalf Of Stephen Smalley
> Sent: Tuesday, June 25, 2019 2:49 PM
>
> On 6/25/19 5:01 PM, Stephen Smalley wrote:
> > On 6/21/19 1:05 PM, Xing, Cedric wrote:
> >>> From: Christopherson, Sean J
> >>> Sent: Wednesday, June 19, 2019 3:24 PM
> >>>
> >>> diff --git a/include/linux/security.h b/include/linux/security.h
> >>> index 6a1f54ba6794..572ddfc53039 100644
> >>> --- a/include/linux/security.h
> >>> +++ b/include/linux/security.h
> >>> @@ -1832,11 +1832,18 @@ static inline void
> >>> security_bpf_prog_free(struct bpf_prog_aux *aux)
> >>> #ifdef CONFIG_INTEL_SGX
> >>> #ifdef CONFIG_SECURITY
> >>> int security_enclave_map(unsigned long prot);
> >>> +int security_enclave_load(struct vm_area_struct *vma, unsigned long
> >>> prot,
> >>> + bool measured);
> >>> #else
> >>> static inline int security_enclave_map(unsigned long prot)
> >>> {
> >>> return 0;
> >>> }
> >>> +static inline int security_enclave_load(struct vm_area_struct *vma,
> >>> + unsigned long prot, bool measured) {
> >>> + return 0;
> >>> +}
> >>> #endif /* CONFIG_SECURITY */
> >>> #endif /* CONFIG_INTEL_SGX */
> >>
> >> Parameters to security_enclave_load() are specific on what's being
> >> loading only, but unspecific on which enclave to be loaded into. That
> >> kills the possibility of an LSM module making enclave dependent
> >> decisions.
> >>
> >> Btw, if enclave (in the form of struct file) is also passed in as a
> >> parameter, it'd let LSM know that file is an enclave, hence would be
> >> able to make the same decision in security_mmap_file() as in
> >> security_enclave_map(). In other words, you wouldn't need
> >> security_enclave_map().
> >
> > Sorry, you want security_enclave_load() to stash a reference to the
> > enclave file in some security module-internal state, then match it
> > upon later security_mmap_file() calls to determine that it is dealing
> > with an enclave, and then adjust its logic accordingly? When do we
> > release that reference?
>
> I guess you mean set a flag in the enclave file security struct upon
> security_enclave_load() and check that flag in security_mmap_file().
Yes, by invoking security_enclave_load(), the SGX subsystem indicates to LSM the file struct in subject refers to an enclave. But security_mmap_file() doesn't pass in the range being mmap()'ed so LSM still cannot decide. Instead of changing the definition of security_mmap_file(), I'd invoke security_file_mprotect() from sgx_mmap(). After all, creating a new mapping is equivalent to changing the target range from PROT_NONE to @prot being requested. I just sent out a patch series with all those details in code.
>
> This seems somewhat similar to one of Sean's alternatives in the patch
> description for 06/12, except by pushing the information from sgx to LSM
> upon security_enclave_load() rather than pulling it via a
> is_sgx_enclave() helper. Not clear if it is still subject to the same
> limitations.
Yes, they are similar except who keeps track of that piece of information. As Dr. Greg pointed out, the new hooks do have to be SGX specific. But calling is_sgx_enclave() really ties LSM to SGX. In contrast, inferring it through security_enclave_load() makes LSM SGX-agnostic. Then the only SGX specific thing left in the hooks is the sgx_sigstruct. In theory, that's just a digital signature and as Dr. Greg pointed out, SGX is probably not the only technology that uses digital signature to identify executable contents. And in that sense, if we rename it to something generic with probably a tag indicating its format, then the whole thing would become SGX agnostic and could be useful for other TEEs on architectures other than x86.
^ permalink raw reply
* Re: [PATCH v9 1/2] mm: security: introduce init_on_alloc=1 and init_on_free=1 boot options
From: James Morris @ 2019-06-27 19:59 UTC (permalink / raw)
To: Alexander Potapenko
Cc: Andrew Morton, Christoph Lameter, Kees Cook, Masahiro Yamada,
Michal Hocko, Serge E. Hallyn, Nick Desaulniers,
Kostya Serebryany, Dmitry Vyukov, Sandeep Patil, Laura Abbott,
Randy Dunlap, Jann Horn, Mark Rutland, Marco Elver, Qian Cai,
linux-mm, linux-security-module, kernel-hardening
In-Reply-To: <20190627130316.254309-2-glider@google.com>
On Thu, 27 Jun 2019, Alexander Potapenko wrote:
> Signed-off-by: Alexander Potapenko <glider@google.com>
> Acked-by: Kees Cook <keescook@chromium.org>
> To: Andrew Morton <akpm@linux-foundation.org>
> To: Christoph Lameter <cl@linux.com>
> To: Kees Cook <keescook@chromium.org>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: James Morris <jmorris@namei.org>
> Cc: "Serge E. Hallyn" <serge@hallyn.com>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Kostya Serebryany <kcc@google.com>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> Cc: Sandeep Patil <sspatil@android.com>
> Cc: Laura Abbott <labbott@redhat.com>
> Cc: Randy Dunlap <rdunlap@infradead.org>
> Cc: Jann Horn <jannh@google.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Marco Elver <elver@google.com>
> Cc: Qian Cai <cai@lca.pw>
> Cc: linux-mm@kvack.org
> Cc: linux-security-module@vger.kernel.org
> Cc: kernel-hardening@lists.openwall.com
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Acked-by: James Morris <jamorris@linux.microsoft.com>
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH V33 24/30] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: Stephen Smalley @ 2019-06-27 20:16 UTC (permalink / raw)
To: James Morris
Cc: Andy Lutomirski, Andy Lutomirski, Matthew Garrett, linux-security,
LKML, Linux API, David Howells, Alexei Starovoitov,
Matthew Garrett, Network Development, Chun-Yi Lee,
Daniel Borkmann, linux-security-module
In-Reply-To: <alpine.LRH.2.21.1906280332500.17363@namei.org>
On 6/27/19 2:06 PM, James Morris wrote:
> On Thu, 27 Jun 2019, Stephen Smalley wrote:
>
>> There are two scenarios where finer-grained distinctions make sense:
>>
>> - Users may need to enable specific functionality that falls under the
>> umbrella of "confidentiality" or "integrity" lockdown. Finer-grained lockdown
>> reasons free them from having to make an all-or-nothing choice between lost
>> functionality or no lockdown at all.
>
> Agreed. This will be used for more than just UEFI secure boot on desktops,
> e.g. embedded systems using verified boot, where finer grained policy will
> be needed for what are sometimes very specific use-cases (which may be
> also covered by other mitigations).
>
>> This can be supported directly by the
>> lockdown module without any help from SELinux or other security modules; we
>> just need the ability to specify these finer-grained lockdown levels via the
>> boot parameters and securityfs nodes.
>
> If the lockdown LSM implements fine grained policy (rather than the simple
> coarse grained policy), I'd suggest adding a new lockdown level of
> 'custom' which by default enables all hooks but allows selective
> disablement via params/sysfs.
>
> This would be simpler than telling users to use a different lockdown LSM
> for this.
>
>> - Different processes/programs may need to use different sets of functionality
>> restricted via lockdown confidentiality or integrity categories. If we have
>> to allow all-or-none for the set of interfaces/functionality covered by the
>> generic confidentiality or integrity categories, then we'll end up having to
>> choose between lost functionality or overprivileged processes, neither of
>> which is optimal.
>>
>> Is it truly the case that everything under the "confidentiality" category
>> poses the same level of risk to kernel confidentiality, and similarly for
>> everything under the "integrity" category? If not, then being able to
>> distinguish them definitely has benefit.
>
> Good question. We can't know the answer to this unless we know how an
> attacker might leverage access.
>
> The value here IMHO is more in allowing tradeoffs to be made by system
> designers vs. disabling lockdown entirely.
>
>> I'm still not clear though on how/if this will compose with or be overridden
>> by other security modules. We would need some means for another security
>> module to take over lockdown decisions once it has initialized (including
>> policy load), and to be able to access state that is currently private to the
>> lockdown module, like the level.
>
> Why not utilize stacking (restrictively), similarly to capabilities?
That would only allow the LSM to further lock down the system above the
lockdown level set at boot, not grant exemptions for specific
functionality/interfaces required by the user or by a specific
process/program. You'd have to boot with lockdown=none (or your
lockdown=custom suggestion) in order for the LSM to allow anything
covered by the integrity or confidentiality levels. And then the kernel
would be unprotected prior to full initialization of the LSM, including
policy load.
It seems like one would want to be able to boot with lockdown=integrity
to protect the kernel initially, then switch over to allowing the LSM to
selectively override it.
^ permalink raw reply
* RE: [RFC PATCH v4 10/12] security/selinux: Add enclave_load() implementation
From: Xing, Cedric @ 2019-06-27 20:19 UTC (permalink / raw)
To: Stephen Smalley, Christopherson, Sean J, Jarkko Sakkinen
Cc: linux-sgx@vger.kernel.org, linux-security-module@vger.kernel.org,
selinux@vger.kernel.org, Roberts, William C, Schaufler, Casey,
James Morris, Hansen, Dave, Andy Lutomirski, Jethro Beekman,
Dr . Greg Wettstein
In-Reply-To: <b36d6fd0-3135-e48d-ed84-d69853bd79f1@tycho.nsa.gov>
> From: linux-sgx-owner@vger.kernel.org [mailto:linux-sgx-
> owner@vger.kernel.org] On Behalf Of Stephen Smalley
> Sent: Tuesday, June 25, 2019 2:10 PM
>
> On 6/21/19 5:22 PM, Xing, Cedric wrote:
> >> From: Christopherson, Sean J
> >> Sent: Wednesday, June 19, 2019 3:24 PM
> >>
> >> Intended use of each permission:
> >>
> >> - SGX_EXECDIRTY: dynamically load code within the enclave itself
> >> - SGX_EXECUNMR: load unmeasured code into the enclave, e.g.
> >> Graphene
> >
> > Why does it matter whether a code page is measured or not?
>
> It won't be incorporated into an attestation?
Yes, it will. And because of that, I don't think LSM should care.
>
> >
> >> - SGX_EXECANON: load code from anonymous memory (likely Graphene)
> >
> > Graphene doesn't load code from anonymous memory. It loads code
> dynamically though, as in SGX_EXECDIRTY case.
>
> So do we expect EXECANON to never be triggered at all?
I don't think so. And from security perspective, the decision I think shall base on whether the source pages are (allowed to be made) executable.
>
> >
> >> - SGX_EXECUTE: load an enclave from a file, i.e. normal behavior
> >
> > Why is SGX_EXECUTE needed from security perspective? Or why isn't
> FILE__EXECUTE sufficient?
>
> Splitting the SGX permissions from the regular ones allows distinctions
> to be made between what can be executed in the host process and what can
> be executed in the enclave. The host process may be allowed
> FILE__EXECUTE to numerous files that do not contain any code ever
> intended to be executed within the enclave.
Given an enclave and its host process, any executable contents could be allowed in
1) Neither the enclave nor the host
2) Enclave only
3) Host only
4) Both the enclave and the host
Given the fact that enclave can access host's memory, if a piece of code is NOT allowed in the host, then it shouldn't be allowed in enclave either. So #2 shall never happen.
An enclave dictates/enforces its own contents cryptographically, so it's unnecessary to enforce #3 by LSM IMO.
Then #1 and #4 are the only 2 cases to be supported - a single FILE__EXECUTE is sufficient.
I'm not objecting to new permissions to make things more explicit, but that'd require updates to user mode tools. I think it just easier to reuse existing permissions.
^ permalink raw reply
* RE: [RFC PATCH v4 08/12] security/selinux: Require SGX_MAPWX to map enclave page WX
From: Xing, Cedric @ 2019-06-27 20:26 UTC (permalink / raw)
To: Stephen Smalley, Christopherson, Sean J, Jarkko Sakkinen
Cc: linux-sgx@vger.kernel.org, linux-security-module@vger.kernel.org,
selinux@vger.kernel.org, Roberts, William C, Schaufler, Casey,
James Morris, Hansen, Dave, Andy Lutomirski, Jethro Beekman,
Dr . Greg Wettstein
In-Reply-To: <6f86681a-5478-c46c-b74a-52a973b53320@tycho.nsa.gov>
> From: linux-sgx-owner@vger.kernel.org [mailto:linux-sgx-
> owner@vger.kernel.org] On Behalf Of Stephen Smalley
> Sent: Tuesday, June 25, 2019 2:06 PM
>
> On 6/21/19 1:09 PM, Xing, Cedric wrote:
> >> From: Christopherson, Sean J
> >> Sent: Wednesday, June 19, 2019 3:24 PM
> >>
> >> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> >> index 3ec702cf46ca..fc239e541b62 100644
> >> --- a/security/selinux/hooks.c
> >> +++ b/security/selinux/hooks.c
> >> @@ -6726,6 +6726,23 @@ static void selinux_bpf_prog_free(struct
> bpf_prog_aux *aux)
> >> }
> >> #endif
> >>
> >> +#ifdef CONFIG_INTEL_SGX
> >> +static int selinux_enclave_map(unsigned long prot) {
> >> + const struct cred *cred = current_cred();
> >> + u32 sid = cred_sid(cred);
> >> +
> >> + /* SGX is supported only in 64-bit kernels. */
> >> + WARN_ON_ONCE(!default_noexec);
> >> +
> >> + if ((prot & PROT_EXEC) && (prot & PROT_WRITE))
> >> + return avc_has_perm(&selinux_state, sid, sid,
> >> + SECCLASS_PROCESS2, PROCESS2__SGX_MAPWX,
> >> + NULL);
> >
> > Why isn't SGX_MAPWX enclave specific but process wide?
>
> How would you tie it to a specific enclave? What's the object/target
> SID? The SID of the enclave inode? Which one? The source vma file,
> the /dev/sgx/enclave open instance, the sigstruct file, ...? If a
> process can map one enclave WX, what's the benefit of preventing it from
> doing likewise for any other enclave it can load?
I wasn't saying we should. Rather, I think we can reuse EXECMEM. After all, under what circumstances are WX necessary? IMHO, WX shall be strongly discouraged and this SGX_MAPWX is kind of trying to give the bearing enclave a dirty look. And if that's the sole purpose, let's make it even dirtier by requiring EXECMEM on the host process. After all, WX is never a good thing in security so I doubt any ISVs would have a practical reason to require WX in their enclaves.
^ permalink raw reply
* RE: [RFC PATCH v4 07/12] LSM: x86/sgx: Introduce ->enclave_map() hook for Intel SGX
From: Xing, Cedric @ 2019-06-27 20:29 UTC (permalink / raw)
To: Stephen Smalley, Christopherson, Sean J, Jarkko Sakkinen
Cc: linux-sgx@vger.kernel.org, linux-security-module@vger.kernel.org,
selinux@vger.kernel.org, Roberts, William C, Schaufler, Casey,
James Morris, Hansen, Dave, Andy Lutomirski, Jethro Beekman,
Dr . Greg Wettstein
In-Reply-To: <8235709b-9034-4751-30ce-720e41e31525@tycho.nsa.gov>
> From: linux-sgx-owner@vger.kernel.org [mailto:linux-sgx-
> owner@vger.kernel.org] On Behalf Of Stephen Smalley
> Sent: Tuesday, June 25, 2019 1:48 PM
>
> On 6/21/19 12:54 PM, Xing, Cedric wrote:
> >> From: Christopherson, Sean J
> >> Sent: Wednesday, June 19, 2019 3:24 PM
> >>
> >> diff --git a/security/security.c b/security/security.c index
> >> 613a5c00e602..03951e08bdfc 100644
> >> --- a/security/security.c
> >> +++ b/security/security.c
> >> @@ -2359,3 +2359,10 @@ void security_bpf_prog_free(struct
> bpf_prog_aux *aux)
> >> call_void_hook(bpf_prog_free_security, aux);
> >> }
> >> #endif /* CONFIG_BPF_SYSCALL */
> >> +
> >> +#ifdef CONFIG_INTEL_SGX
> >> +int security_enclave_map(unsigned long prot) {
> >> + return call_int_hook(enclave_map, 0, prot); } #endif /*
> >> +CONFIG_INTEL_SGX */
> >
> > Why is this new security_enclave_map() necessary while
> security_mmap_file() will also be invoked?
>
> security_mmap_file() doesn't know about enclaves. It will just end up
> checking FILE__READ, FILE__WRITE, and FILE__EXECUTE to /dev/sgx/enclave.
> This was noted in the patch description.
Surely I understand all those. As I mentioned in my other email, enclave_load() could indicate to LSM that a file is an enclave. Of course mmap() could be invoked before any pages are loaded so LSM wouldn't know at the first mmap(), but that doesn't matter as an empty enclave wouldn't post any threats anyway.
^ permalink raw reply
* Re: [PATCH 8/9] keys: Network namespace domain tag [ver #4]
From: Willem de Bruijn @ 2019-06-27 20:55 UTC (permalink / raw)
To: David Howells
Cc: Eric W. Biederman, keyrings, linux-cifs, linux-nfs,
Network Development, linux-afs, dwalsh, vgoyal,
linux-security-module, linux-fsdevel, LKML
In-Reply-To: <156096287188.28733.15342608110117616221.stgit@warthog.procyon.org.uk>
On Wed, Jun 19, 2019 at 12:49 PM David Howells <dhowells@redhat.com> wrote:
>
> Create key domain tags for network namespaces and make it possible to
> automatically tag keys that are used by networked services (e.g. AF_RXRPC,
> AFS, DNS) with the default network namespace if not set by the caller.
>
> This allows keys with the same description but in different namespaces to
> coexist within a keyring.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: netdev@vger.kernel.org
> cc: linux-nfs@vger.kernel.org
> cc: linux-cifs@vger.kernel.org
> cc: linux-afs@lists.infradead.org
> ---
> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
> index 711b161505ac..076a75c73c9e 100644
> --- a/net/core/net_namespace.c
> +++ b/net/core/net_namespace.c
> @@ -38,9 +38,16 @@ EXPORT_SYMBOL_GPL(net_namespace_list);
> DECLARE_RWSEM(net_rwsem);
> EXPORT_SYMBOL_GPL(net_rwsem);
>
> +#ifdef CONFIG_KEYS
> +static struct key_tag init_net_key_domain = { .usage = REFCOUNT_INIT(1) };
> +#endif
> +
> struct net init_net = {
> .count = REFCOUNT_INIT(1),
> .dev_base_head = LIST_HEAD_INIT(init_net.dev_base_head),
> +#ifdef CONFIG_KEYS
> + .key_domain = &init_net_key_domain,
> +#endif
> };
> EXPORT_SYMBOL(init_net);
>
> @@ -386,10 +393,21 @@ static struct net *net_alloc(void)
> if (!net)
> goto out_free;
>
> +#ifdef CONFIG_KEYS
> + net->key_domain = kzalloc(sizeof(struct key_tag), GFP_KERNEL);
> + if (!net->key_domain)
> + goto out_free_2;
> + refcount_set(&net->key_domain->usage, 1);
> +#endif
> +
> rcu_assign_pointer(net->gen, ng);
> out:
> return net;
>
> +#ifdef CONFIG_KEYS
> +out_free_2:
> + kmem_cache_free(net_cachep, net);
needs
net = NULL;
to signal failure
> +#endif
> out_free:
> kfree(ng);
> goto out;
Reported-by: syzbot <syzkaller@googlegroups.com>
BUG: KASAN: use-after-free in atomic_set
include/asm-generic/atomic-instrumented.h:44 [inline]
BUG: KASAN: use-after-free in refcount_set include/linux/refcount.h:32 [inline]
BUG: KASAN: use-after-free in copy_net_ns+0x1e8/0x431
net/core/net_namespace.c:466
Write of size 4 at addr ffff88809c9de080 by task syz-executor.1/12624
CPU: 1 PID: 12624 Comm: syz-executor.1 Not tainted 5.2.0-rc6-next-20190626 #23
Hardware name: Google Google Compute Engine/Google Compute Engine,
BIOS Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x172/0x1f0 lib/dump_stack.c:113
print_address_description.cold+0xd4/0x306 mm/kasan/report.c:351
__kasan_report.cold+0x1b/0x36 mm/kasan/report.c:482
kasan_report+0x12/0x17 mm/kasan/common.c:614
check_memory_region_inline mm/kasan/generic.c:185 [inline]
check_memory_region+0x123/0x190 mm/kasan/generic.c:191
kasan_check_write+0x14/0x20 mm/kasan/common.c:100
atomic_set include/asm-generic/atomic-instrumented.h:44 [inline]
refcount_set include/linux/refcount.h:32 [inline]
copy_net_ns+0x1e8/0x431 net/core/net_namespace.c:466
create_new_namespaces+0x400/0x7b0 kernel/nsproxy.c:103
unshare_nsproxy_namespaces+0xc2/0x200 kernel/nsproxy.c:202
ksys_unshare+0x444/0x980 kernel/fork.c:2828
__do_sys_unshare kernel/fork.c:2896 [inline]
__se_sys_unshare kernel/fork.c:2894 [inline]
__x64_sys_unshare+0x31/0x40 kernel/fork.c:2894
do_syscall_64+0xfd/0x6a0 arch/x86/entry/common.c:301
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x459519
Code: fd b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48
89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d
01 f0 ff ff 0f 83 cb b7 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007f2202261c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000110
RAX: ffffffffffffffda RBX: 00007f2202261c90 RCX: 0000000000459519
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000040000000
RBP: 000000000075bf20 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00007f22022626d4
R13: 00000000004c8a2c R14: 00000000004df7d0 R15: 0000000000000006
Allocated by task 12624:
save_stack+0x23/0x90 mm/kasan/common.c:71
set_track mm/kasan/common.c:79 [inline]
__kasan_kmalloc mm/kasan/common.c:489 [inline]
__kasan_kmalloc.constprop.0+0xcf/0xe0 mm/kasan/common.c:462
kasan_slab_alloc+0xf/0x20 mm/kasan/common.c:497
slab_post_alloc_hook mm/slab.h:520 [inline]
slab_alloc mm/slab.c:3320 [inline]
kmem_cache_alloc+0x121/0x710 mm/slab.c:3484
kmem_cache_zalloc include/linux/slab.h:737 [inline]
net_alloc net/core/net_namespace.c:410 [inline]
copy_net_ns+0xf1/0x431 net/core/net_namespace.c:461
create_new_namespaces+0x400/0x7b0 kernel/nsproxy.c:103
unshare_nsproxy_namespaces+0xc2/0x200 kernel/nsproxy.c:202
ksys_unshare+0x444/0x980 kernel/fork.c:2828
__do_sys_unshare kernel/fork.c:2896 [inline]
__se_sys_unshare kernel/fork.c:2894 [inline]
__x64_sys_unshare+0x31/0x40 kernel/fork.c:2894
do_syscall_64+0xfd/0x6a0 arch/x86/entry/common.c:301
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Freed by task 12624:
save_stack+0x23/0x90 mm/kasan/common.c:71
set_track mm/kasan/common.c:79 [inline]
__kasan_slab_free+0x102/0x150 mm/kasan/common.c:451
kasan_slab_free+0xe/0x10 mm/kasan/common.c:459
__cache_free mm/slab.c:3426 [inline]
kmem_cache_free+0x86/0x320 mm/slab.c:3694
net_alloc net/core/net_namespace.c:427 [inline]
copy_net_ns+0x3b1/0x431 net/core/net_namespace.c:461
create_new_namespaces+0x400/0x7b0 kernel/nsproxy.c:103
unshare_nsproxy_namespaces+0xc2/0x200 kernel/nsproxy.c:202
ksys_unshare+0x444/0x980 kernel/fork.c:2828
__do_sys_unshare kernel/fork.c:2896 [inline]
__se_sys_unshare kernel/fork.c:2894 [inline]
__x64_sys_unshare+0x31/0x40 kernel/fork.c:2894
do_syscall_64+0xfd/0x6a0 arch/x86/entry/common.c:301
entry_SYSCALL_64_after_hwframe+0x49/0xbe
^ permalink raw reply
* Re: [PATCH v4 15/23] LSM: Specify which LSM to display
From: John Johansen @ 2019-06-27 21:33 UTC (permalink / raw)
To: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
selinux
Cc: keescook, penguin-kernel, paul, sds
In-Reply-To: <20190626192234.11725-16-casey@schaufler-ca.com>
On 6/26/19 12:22 PM, Casey Schaufler wrote:
> Create a new entry "display" in /proc/.../attr for controlling
> which LSM security information is displayed for a process.
> The name of an active LSM that supplies hooks for human readable
> data may be written to "display" to set the value. The name of
> the LSM currently in use can be read from "display".
> At this point there can only be one LSM capable of display
> active.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Reviewed-by: John Johansen <john.johansen@canonical.com>
> ---
> fs/proc/base.c | 1 +
> security/security.c | 129 ++++++++++++++++++++++++++++++++++++++------
> 2 files changed, 113 insertions(+), 17 deletions(-)
>
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index ddef482f1334..7bf70e041315 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -2618,6 +2618,7 @@ static const struct pid_entry attr_dir_stuff[] = {
> ATTR(NULL, "fscreate", 0666),
> ATTR(NULL, "keycreate", 0666),
> ATTR(NULL, "sockcreate", 0666),
> + ATTR(NULL, "display", 0666),
> #ifdef CONFIG_SECURITY_SMACK
> DIR("smack", 0555,
> proc_smack_attr_dir_inode_ops, proc_smack_attr_dir_ops),
> diff --git a/security/security.c b/security/security.c
> index 3180a6f30625..82e29c477fa4 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -46,7 +46,9 @@ static struct kmem_cache *lsm_file_cache;
> static struct kmem_cache *lsm_inode_cache;
>
> char *lsm_names;
> -static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init;
> +static struct lsm_blob_sizes blob_sizes __lsm_ro_after_init = {
> + .lbs_task = sizeof(int), /* slot number for the "display" LSM */
> +};
>
> /* Boot-time LSM user choice */
> static __initdata const char *chosen_lsm_order;
> @@ -423,8 +425,10 @@ static int lsm_append(const char *new, char **result)
>
> /*
> * Current index to use while initializing the lsmblob secid list.
> + * Pointers to the LSM id structures for local use.
> */
> static int lsm_slot;
> +static struct lsm_id *lsm_slotlist[LSMBLOB_ENTRIES];
>
> /**
> * security_add_hooks - Add a modules hooks to the hook lists.
> @@ -444,6 +448,7 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
> if (lsmid->slot == LSMBLOB_NEEDED) {
> if (lsm_slot >= LSMBLOB_ENTRIES)
> panic("%s Too many LSMs registered.\n", __func__);
> + lsm_slotlist[lsm_slot] = lsmid;
> lsmid->slot = lsm_slot++;
> init_debug("%s assigned lsmblob slot %d\n", lsmid->lsm,
> lsmid->slot);
> @@ -564,6 +569,8 @@ int lsm_inode_alloc(struct inode *inode)
> */
> static int lsm_task_alloc(struct task_struct *task)
> {
> + int *display;
> +
> if (blob_sizes.lbs_task == 0) {
> task->security = NULL;
> return 0;
> @@ -572,6 +579,15 @@ static int lsm_task_alloc(struct task_struct *task)
> task->security = kzalloc(blob_sizes.lbs_task, GFP_KERNEL);
> if (task->security == NULL)
> return -ENOMEM;
> +
> + /*
> + * The start of the task blob contains the "display" LSM slot number.
> + * Start with it set to the invalid slot number, indicating that the
> + * default first registered LSM be displayed.
> + */
> + display = task->security;
> + *display = LSMBLOB_INVALID;
> +
> return 0;
> }
>
> @@ -1563,14 +1579,24 @@ int security_file_open(struct file *file)
>
> int security_task_alloc(struct task_struct *task, unsigned long clone_flags)
> {
> + int *odisplay = current->security;
> + int *ndisplay;
> int rc = lsm_task_alloc(task);
>
> - if (rc)
> + if (unlikely(rc))
> return rc;
> +
> rc = call_int_hook(task_alloc, 0, task, clone_flags);
> - if (unlikely(rc))
> + if (unlikely(rc)) {
> security_task_free(task);
> - return rc;
> + return rc;
> + }
> +
> + ndisplay = task->security;
> + if (ndisplay && odisplay)
> + *ndisplay = *odisplay;
> +
> + return 0;
> }
>
> void security_task_free(struct task_struct *task)
> @@ -1967,10 +1993,29 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name,
> char **value)
> {
> struct security_hook_list *hp;
> + int *display = current->security;
> + int slot = 0;
> +
> + if (!strcmp(name, "display")) {
> + /*
> + * lsm_slot will be 0 if there are no displaying modules.
> + */
> + if (lsm_slot == 0)
> + return -EINVAL;
> + if (*display != LSMBLOB_INVALID)
> + slot = *display;
> + *value = kstrdup(lsm_slotlist[slot]->lsm, GFP_KERNEL);
> + if (*value)
> + return strlen(*value);
> + return -ENOMEM;
> + }
>
> hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) {
> if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
> continue;
> + if (lsm == NULL && *display != LSMBLOB_INVALID &&
> + *display != hp->lsmid->slot)
> + continue;
> return hp->hook.getprocattr(p, name, value);
> }
> return -EINVAL;
> @@ -1980,10 +2025,46 @@ int security_setprocattr(const char *lsm, const char *name, void *value,
> size_t size)
> {
> struct security_hook_list *hp;
> + char *term;
> + char *cp;
> + int *display = current->security;
> + int rc = -EINVAL;
> + int slot = 0;
> +
> + if (!strcmp(name, "display")) {
> + /*
> + * lsm_slot will be 0 if there are no displaying modules.
> + */
> + if (lsm_slot == 0 || size == 0)
> + return -EINVAL;
> + cp = kzalloc(size + 1, GFP_KERNEL);
> + if (cp == NULL)
> + return -ENOMEM;
> + memcpy(cp, value, size);
> +
> + term = strchr(cp, ' ');
> + if (term == NULL)
> + term = strchr(cp, '\n');
> + if (term != NULL)
> + *term = '\0';
> +
> + for (slot = 0; slot < lsm_slot; slot++)
> + if (!strcmp(cp, lsm_slotlist[slot]->lsm)) {
> + *display = lsm_slotlist[slot]->slot;
> + rc = size;
> + break;
> + }
> +
> + kfree(cp);
> + return rc;
> + }
>
> hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) {
> if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
> continue;
> + if (lsm == NULL && *display != LSMBLOB_INVALID &&
> + *display != hp->lsmid->slot)
> + continue;
> return hp->hook.setprocattr(name, value, size);
> }
> return -EINVAL;
> @@ -2003,15 +2084,15 @@ EXPORT_SYMBOL(security_ismaclabel);
> int security_secid_to_secctx(struct lsmblob *blob, char **secdata, u32 *seclen)
> {
> struct security_hook_list *hp;
> - int rc;
> + int *display = current->security;
>
> hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list) {
> if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
> continue;
> - rc = hp->hook.secid_to_secctx(blob->secid[hp->lsmid->slot],
> - secdata, seclen);
> - if (rc != 0)
> - return rc;
> + if (*display == LSMBLOB_INVALID || *display == hp->lsmid->slot)
> + return hp->hook.secid_to_secctx(
> + blob->secid[hp->lsmid->slot],
> + secdata, seclen);
> }
> return 0;
> }
> @@ -2021,16 +2102,15 @@ int security_secctx_to_secid(const char *secdata, u32 seclen,
> struct lsmblob *blob)
> {
> struct security_hook_list *hp;
> - int rc;
> + int *display = current->security;
>
> lsmblob_init(blob, 0);
> hlist_for_each_entry(hp, &security_hook_heads.secctx_to_secid, list) {
> if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
> continue;
> - rc = hp->hook.secctx_to_secid(secdata, seclen,
> - &blob->secid[hp->lsmid->slot]);
> - if (rc != 0)
> - return rc;
> + if (*display == LSMBLOB_INVALID || *display == hp->lsmid->slot)
> + return hp->hook.secctx_to_secid(secdata, seclen,
> + &blob->secid[hp->lsmid->slot]);
> }
> return 0;
> }
> @@ -2038,7 +2118,15 @@ EXPORT_SYMBOL(security_secctx_to_secid);
>
> void security_release_secctx(char *secdata, u32 seclen)
> {
> - call_void_hook(release_secctx, secdata, seclen);
> + struct security_hook_list *hp;
> + int *display = current->security;
> +
> + hlist_for_each_entry(hp, &security_hook_heads.release_secctx, list)
> + if (*display == LSMBLOB_INVALID ||
> + *display == hp->lsmid->slot) {
> + hp->hook.release_secctx(secdata, seclen);
> + return;
> + }
> }
> EXPORT_SYMBOL(security_release_secctx);
>
> @@ -2163,8 +2251,15 @@ EXPORT_SYMBOL(security_sock_rcv_skb);
> int security_socket_getpeersec_stream(struct socket *sock, char __user *optval,
> int __user *optlen, unsigned len)
> {
> - return call_int_hook(socket_getpeersec_stream, -ENOPROTOOPT, sock,
> - optval, optlen, len);
> + int *display = current->security;
> + struct security_hook_list *hp;
> +
> + hlist_for_each_entry(hp, &security_hook_heads.socket_getpeersec_stream,
> + list)
> + if (*display == LSMBLOB_INVALID || *display == hp->lsmid->slot)
> + return hp->hook.socket_getpeersec_stream(sock, optval,
> + optlen, len);
> + return -ENOPROTOOPT;
> }
>
> int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb,
>
^ permalink raw reply
* Re: [PATCH v4 16/23] LSM: Use lsmcontext in security_secid_to_secctx
From: John Johansen @ 2019-06-27 21:34 UTC (permalink / raw)
To: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
selinux
Cc: keescook, penguin-kernel, paul, sds
In-Reply-To: <20190626192234.11725-17-casey@schaufler-ca.com>
On 6/26/19 12:22 PM, Casey Schaufler wrote:
> Replace the (secctx,seclen) pointer pair with a single
> lsmcontext pointer to allow return of the LSM identifier
> along with the context and context length. This allows
> security_release_secctx() to know how to release the
> context. Callers have been modified to use or save the
> returned data from the new structure.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Reviewed-by: John Johansen <john.johansen@canonical.com>
> ---
> drivers/android/binder.c | 10 +++++--
> fs/kernfs/dir.c | 9 ++++--
> fs/kernfs/inode.c | 7 +++--
> fs/nfs/nfs4proc.c | 8 +++--
> fs/nfsd/nfs4xdr.c | 7 +++--
> include/linux/security.h | 39 +++++++++++++++++++++++--
> include/net/scm.h | 4 ++-
> kernel/audit.c | 14 ++++++---
> kernel/auditsc.c | 12 ++++++--
> net/ipv4/ip_sockglue.c | 4 ++-
> net/netfilter/nf_conntrack_netlink.c | 4 ++-
> net/netfilter/nf_conntrack_standalone.c | 4 ++-
> net/netfilter/nfnetlink_queue.c | 13 ++++++---
> net/netlabel/netlabel_unlabeled.c | 19 +++++++++---
> net/netlabel/netlabel_user.c | 4 ++-
> security/security.c | 12 ++++----
> security/smack/smack_lsm.c | 14 ++++++---
> 17 files changed, 141 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index 144ac4f1c24f..89e574be34cc 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -2876,6 +2876,7 @@ static void binder_transaction(struct binder_proc *proc,
> int t_debug_id = atomic_inc_return(&binder_last_id);
> char *secctx = NULL;
> u32 secctx_sz = 0;
> + struct lsmcontext scaff; /* scaffolding */
>
> e = binder_transaction_log_add(&binder_transaction_log);
> e->debug_id = t_debug_id;
> @@ -3158,7 +3159,8 @@ static void binder_transaction(struct binder_proc *proc,
> binder_alloc_copy_to_buffer(&target_proc->alloc,
> t->buffer, buf_offset,
> secctx, secctx_sz);
> - security_release_secctx(secctx, secctx_sz);
> + lsmcontext_init(&scaff, secctx, secctx_sz, 0);
> + security_release_secctx(&scaff);
> secctx = NULL;
> }
> t->buffer->debug_id = t->debug_id;
> @@ -3479,8 +3481,10 @@ static void binder_transaction(struct binder_proc *proc,
> t->buffer->transaction = NULL;
> binder_alloc_free_buf(&target_proc->alloc, t->buffer);
> err_binder_alloc_buf_failed:
> - if (secctx)
> - security_release_secctx(secctx, secctx_sz);
> + if (secctx) {
> + lsmcontext_init(&scaff, secctx, secctx_sz, 0);
> + security_release_secctx(&scaff);
> + }
> err_get_secctx_failed:
> kfree(tcomplete);
> binder_stats_deleted(BINDER_STAT_TRANSACTION_COMPLETE);
> diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
> index b84d635567d3..92afad387237 100644
> --- a/fs/kernfs/dir.c
> +++ b/fs/kernfs/dir.c
> @@ -532,9 +532,12 @@ void kernfs_put(struct kernfs_node *kn)
> kfree_const(kn->name);
>
> if (kn->iattr) {
> - if (kn->iattr->ia_secdata)
> - security_release_secctx(kn->iattr->ia_secdata,
> - kn->iattr->ia_secdata_len);
> + struct lsmcontext scaff; /* scaffolding */
> + if (kn->iattr->ia_secdata) {
> + lsmcontext_init(&scaff, kn->iattr->ia_secdata,
> + kn->iattr->ia_secdata_len, 0);
> + security_release_secctx(&scaff);
> + }
> simple_xattrs_free(&kn->iattr->xattrs);
> kmem_cache_free(kernfs_iattrs_cache, kn->iattr);
> }
> diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c
> index 0c1fd945ce42..02cde9dac5ee 100644
> --- a/fs/kernfs/inode.c
> +++ b/fs/kernfs/inode.c
> @@ -349,6 +349,7 @@ static int kernfs_security_xattr_set(const struct xattr_handler *handler,
> {
> struct kernfs_node *kn = inode->i_private;
> struct kernfs_iattrs *attrs;
> + struct lsmcontext context;
> void *secdata;
> u32 secdata_len = 0;
> int error;
> @@ -368,8 +369,10 @@ static int kernfs_security_xattr_set(const struct xattr_handler *handler,
> error = kernfs_node_setsecdata(attrs, &secdata, &secdata_len);
> mutex_unlock(&kernfs_mutex);
>
> - if (secdata)
> - security_release_secctx(secdata, secdata_len);
> + if (secdata) {
> + lsmcontext_init(&context, secdata, secdata_len, 0);
> + security_release_secctx(&context);
> + }
> return error;
> }
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 4dbb0ee23432..af1c0db29c39 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -131,8 +131,12 @@ nfs4_label_init_security(struct inode *dir, struct dentry *dentry,
> static inline void
> nfs4_label_release_security(struct nfs4_label *label)
> {
> - if (label)
> - security_release_secctx(label->label, label->len);
> + struct lsmcontext scaff; /* scaffolding */
> +
> + if (label) {
> + lsmcontext_init(&scaff, label->label, label->len, 0);
> + security_release_secctx(&scaff);
> + }
> }
> static inline u32 *nfs4_bitmask(struct nfs_server *server, struct nfs4_label *label)
> {
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index 3de42a729093..bb3db033e144 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -2420,6 +2420,7 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
> __be32 status;
> int err;
> struct nfs4_acl *acl = NULL;
> + struct lsmcontext scaff; /* scaffolding */
> void *context = NULL;
> int contextlen;
> bool contextsupport = false;
> @@ -2919,8 +2920,10 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
>
> out:
> #ifdef CONFIG_NFSD_V4_SECURITY_LABEL
> - if (context)
> - security_release_secctx(context, contextlen);
> + if (context) {
> + lsmcontext_init(&scaff, context, contextlen, 0); /*scaffolding*/
> + security_release_secctx(&scaff);
> + }
> #endif /* CONFIG_NFSD_V4_SECURITY_LABEL */
> kfree(acl);
> if (tempfh) {
> diff --git a/include/linux/security.h b/include/linux/security.h
> index d310fa3942ce..046012a7255f 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -76,6 +76,41 @@ enum lsm_event {
> LSM_POLICY_CHANGE,
> };
>
> +/*
> + * A "security context" is the text representation of
> + * the information used by LSMs.
> + * This structure contains the string, its length, and which LSM
> + * it is useful for.
> + */
> +struct lsmcontext {
> + char *context; /* Provided by the module */
> + u32 len;
> + int slot; /* Identifies the module */
> +};
> +
> +/**
> + * lsmcontext_init - initialize an lsmcontext structure.
> + * @cp: Pointer to the context to initialize
> + * @context: Initial context, or NULL
> + * @size: Size of context, or 0
> + * @slot: Which LSM provided the context
> + *
> + * Fill in the lsmcontext from the provided information.
> + * This is a scaffolding function that will be removed when
> + * lsmcontext integration is complete.
> + */
> +static inline void lsmcontext_init(struct lsmcontext *cp, char *context,
> + u32 size, int slot)
> +{
> + cp->slot = slot;
> + cp->context = context;
> +
> + if (context == NULL || size == 0)
> + cp->len = 0;
> + else
> + cp->len = strlen(context);
> +}
> +
> /*
> * Data exported by the security modules
> *
> @@ -449,7 +484,7 @@ int security_ismaclabel(const char *name);
> int security_secid_to_secctx(struct lsmblob *blob, char **secdata, u32 *seclen);
> int security_secctx_to_secid(const char *secdata, u32 seclen,
> struct lsmblob *blob);
> -void security_release_secctx(char *secdata, u32 seclen);
> +void security_release_secctx(struct lsmcontext *cp);
>
> void security_inode_invalidate_secctx(struct inode *inode);
> int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen);
> @@ -1240,7 +1275,7 @@ static inline int security_secctx_to_secid(const char *secdata,
> return -EOPNOTSUPP;
> }
>
> -static inline void security_release_secctx(char *secdata, u32 seclen)
> +static inline void security_release_secctx(struct lsmcontext *cp)
> {
> }
>
> diff --git a/include/net/scm.h b/include/net/scm.h
> index 31ae605fcc0a..6c7c3c229e4a 100644
> --- a/include/net/scm.h
> +++ b/include/net/scm.h
> @@ -92,6 +92,7 @@ static __inline__ int scm_send(struct socket *sock, struct msghdr *msg,
> #ifdef CONFIG_SECURITY_NETWORK
> static inline void scm_passec(struct socket *sock, struct msghdr *msg, struct scm_cookie *scm)
> {
> + struct lsmcontext context;
> char *secdata;
> u32 seclen;
> int err;
> @@ -102,7 +103,8 @@ static inline void scm_passec(struct socket *sock, struct msghdr *msg, struct sc
>
> if (!err) {
> put_cmsg(msg, SOL_SOCKET, SCM_SECURITY, seclen, secdata);
> - security_release_secctx(secdata, seclen);
> + lsmcontext_init(&context, secdata, seclen, 0);/*scaffolding*/
> + security_release_secctx(&context);
> }
> }
> }
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 1b51e907f131..f844a2a642e6 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1193,6 +1193,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
> struct audit_sig_info *sig_data;
> char *ctx = NULL;
> u32 len;
> + struct lsmcontext scaff; /* scaffolding */
>
> err = audit_netlink_ok(skb, msg_type);
> if (err)
> @@ -1437,15 +1438,18 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
> }
> sig_data = kmalloc(sizeof(*sig_data) + len, GFP_KERNEL);
> if (!sig_data) {
> - if (lsmblob_is_set(&audit_sig_lsm))
> - security_release_secctx(ctx, len);
> + if (lsmblob_is_set(&audit_sig_lsm)) {
> + lsmcontext_init(&scaff, ctx, len, 0);
> + security_release_secctx(&scaff);
> + }
> return -ENOMEM;
> }
> sig_data->uid = from_kuid(&init_user_ns, audit_sig_uid);
> sig_data->pid = audit_sig_pid;
> if (lsmblob_is_set(&audit_sig_lsm)) {
> memcpy(sig_data->ctx, ctx, len);
> - security_release_secctx(ctx, len);
> + lsmcontext_init(&scaff, ctx, len, 0);
> + security_release_secctx(&scaff);
> }
> audit_send_reply(skb, seq, AUDIT_SIGNAL_INFO, 0, 0,
> sig_data, sizeof(*sig_data) + len);
> @@ -2074,6 +2078,7 @@ int audit_log_task_context(struct audit_buffer *ab)
> unsigned len;
> int error;
> struct lsmblob blob;
> + struct lsmcontext scaff; /* scaffolding */
>
> security_task_getsecid(current, &blob);
> if (!lsmblob_is_set(&blob))
> @@ -2087,7 +2092,8 @@ int audit_log_task_context(struct audit_buffer *ab)
> }
>
> audit_log_format(ab, " subj=%s", ctx);
> - security_release_secctx(ctx, len);
> + lsmcontext_init(&scaff, ctx, len, 0);
> + security_release_secctx(&scaff);
> return 0;
>
> error_path:
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index c7aa39bda5cc..9fab0e7d90c3 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -943,6 +943,7 @@ static int audit_log_pid_context(struct audit_context *context, pid_t pid,
> struct lsmblob *blob, char *comm)
> {
> struct audit_buffer *ab;
> + struct lsmcontext lsmcxt;
> char *ctx = NULL;
> u32 len;
> int rc = 0;
> @@ -960,7 +961,8 @@ static int audit_log_pid_context(struct audit_context *context, pid_t pid,
> rc = 1;
> } else {
> audit_log_format(ab, " obj=%s", ctx);
> - security_release_secctx(ctx, len);
> + lsmcontext_init(&lsmcxt, ctx, len, 0); /*scaffolding*/
> + security_release_secctx(&lsmcxt);
> }
> }
> audit_log_format(ab, " ocomm=");
> @@ -1172,6 +1174,7 @@ static void audit_log_fcaps(struct audit_buffer *ab, struct audit_names *name)
>
> static void show_special(struct audit_context *context, int *call_panic)
> {
> + struct lsmcontext lsmcxt;
> struct audit_buffer *ab;
> int i;
>
> @@ -1205,7 +1208,8 @@ static void show_special(struct audit_context *context, int *call_panic)
> *call_panic = 1;
> } else {
> audit_log_format(ab, " obj=%s", ctx);
> - security_release_secctx(ctx, len);
> + lsmcontext_init(&lsmcxt, ctx, len, 0);
> + security_release_secctx(&lsmcxt);
> }
> }
> if (context->ipc.has_perm) {
> @@ -1352,6 +1356,7 @@ static void audit_log_name(struct audit_context *context, struct audit_names *n,
> char *ctx = NULL;
> u32 len;
> struct lsmblob blob;
> + struct lsmcontext lsmcxt;
>
> lsmblob_init(&blob, n->osid);
> if (security_secid_to_secctx(&blob, &ctx, &len)) {
> @@ -1360,7 +1365,8 @@ static void audit_log_name(struct audit_context *context, struct audit_names *n,
> *call_panic = 2;
> } else {
> audit_log_format(ab, " obj=%s", ctx);
> - security_release_secctx(ctx, len);
> + lsmcontext_init(&lsmcxt, ctx, len, 0); /* scaffolding */
> + security_release_secctx(&lsmcxt);
> }
> }
>
> diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
> index e05f4ef68bd8..7834c357b60b 100644
> --- a/net/ipv4/ip_sockglue.c
> +++ b/net/ipv4/ip_sockglue.c
> @@ -130,6 +130,7 @@ static void ip_cmsg_recv_checksum(struct msghdr *msg, struct sk_buff *skb,
>
> static void ip_cmsg_recv_security(struct msghdr *msg, struct sk_buff *skb)
> {
> + struct lsmcontext context;
> struct lsmblob lb;
> char *secdata;
> u32 seclen;
> @@ -144,7 +145,8 @@ static void ip_cmsg_recv_security(struct msghdr *msg, struct sk_buff *skb)
> return;
>
> put_cmsg(msg, SOL_IP, SCM_SECURITY, seclen, secdata);
> - security_release_secctx(secdata, seclen);
> + lsmcontext_init(&context, secdata, seclen, 0); /* scaffolding */
> + security_release_secctx(&context);
> }
>
> static void ip_cmsg_recv_dstaddr(struct msghdr *msg, struct sk_buff *skb)
> diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
> index ca0968f13240..6954e6600583 100644
> --- a/net/netfilter/nf_conntrack_netlink.c
> +++ b/net/netfilter/nf_conntrack_netlink.c
> @@ -331,6 +331,7 @@ static int ctnetlink_dump_secctx(struct sk_buff *skb, const struct nf_conn *ct)
> int len, ret;
> char *secctx;
> struct lsmblob blob;
> + struct lsmcontext context;
>
> lsmblob_init(&blob, ct->secmark);
> ret = security_secid_to_secctx(&blob, &secctx, &len);
> @@ -348,7 +349,8 @@ static int ctnetlink_dump_secctx(struct sk_buff *skb, const struct nf_conn *ct)
>
> ret = 0;
> nla_put_failure:
> - security_release_secctx(secctx, len);
> + lsmcontext_init(&context, secctx, len, 0); /* scaffolding */
> + security_release_secctx(&context);
> return ret;
> }
> #else
> diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
> index c793103f3cd7..79158ad0486e 100644
> --- a/net/netfilter/nf_conntrack_standalone.c
> +++ b/net/netfilter/nf_conntrack_standalone.c
> @@ -176,6 +176,7 @@ static void ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
> u32 len;
> char *secctx;
> struct lsmblob blob;
> + struct lsmcontext context;
>
> lsmblob_init(&blob, ct->secmark);
> ret = security_secid_to_secctx(&blob, &secctx, &len);
> @@ -184,7 +185,8 @@ static void ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
>
> seq_printf(s, "secctx=%s ", secctx);
>
> - security_release_secctx(secctx, len);
> + lsmcontext_init(&context, secctx, len, 0); /* scaffolding */
> + security_release_secctx(&context);
> }
> #else
> static inline void ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
> diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
> index 59211bff90ab..fe8403ef4e89 100644
> --- a/net/netfilter/nfnetlink_queue.c
> +++ b/net/netfilter/nfnetlink_queue.c
> @@ -399,6 +399,7 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
> enum ip_conntrack_info uninitialized_var(ctinfo);
> struct nfnl_ct_hook *nfnl_ct;
> bool csum_verify;
> + struct lsmcontext scaff; /* scaffolding */
> char *secdata = NULL;
> u32 seclen = 0;
>
> @@ -629,8 +630,10 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
> }
>
> nlh->nlmsg_len = skb->len;
> - if (seclen)
> - security_release_secctx(secdata, seclen);
> + if (seclen) {
> + lsmcontext_init(&scaff, secdata, seclen, 0);
> + security_release_secctx(&scaff);
> + }
> return skb;
>
> nla_put_failure:
> @@ -638,8 +641,10 @@ nfqnl_build_packet_message(struct net *net, struct nfqnl_instance *queue,
> kfree_skb(skb);
> net_err_ratelimited("nf_queue: error creating packet message\n");
> nlmsg_failure:
> - if (seclen)
> - security_release_secctx(secdata, seclen);
> + if (seclen) {
> + lsmcontext_init(&scaff, secdata, seclen, 0);
> + security_release_secctx(&scaff);
> + }
> return NULL;
> }
>
> diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
> index 2294aa9471e6..15b1945853be 100644
> --- a/net/netlabel/netlabel_unlabeled.c
> +++ b/net/netlabel/netlabel_unlabeled.c
> @@ -387,6 +387,7 @@ int netlbl_unlhsh_add(struct net *net,
> struct net_device *dev;
> struct netlbl_unlhsh_iface *iface;
> struct audit_buffer *audit_buf = NULL;
> + struct lsmcontext context;
> char *secctx = NULL;
> u32 secctx_len;
> struct lsmblob blob;
> @@ -457,7 +458,9 @@ int netlbl_unlhsh_add(struct net *net,
> &secctx,
> &secctx_len) == 0) {
> audit_log_format(audit_buf, " sec_obj=%s", secctx);
> - security_release_secctx(secctx, secctx_len);
> + /* scaffolding */
> + lsmcontext_init(&context, secctx, secctx_len, 0);
> + security_release_secctx(&context);
> }
> audit_log_format(audit_buf, " res=%u", ret_val == 0 ? 1 : 0);
> audit_log_end(audit_buf);
> @@ -488,6 +491,7 @@ static int netlbl_unlhsh_remove_addr4(struct net *net,
> struct netlbl_unlhsh_addr4 *entry;
> struct audit_buffer *audit_buf;
> struct net_device *dev;
> + struct lsmcontext context;
> char *secctx;
> u32 secctx_len;
> struct lsmblob blob;
> @@ -516,7 +520,9 @@ static int netlbl_unlhsh_remove_addr4(struct net *net,
> security_secid_to_secctx(&blob,
> &secctx, &secctx_len) == 0) {
> audit_log_format(audit_buf, " sec_obj=%s", secctx);
> - security_release_secctx(secctx, secctx_len);
> + /* scaffolding */
> + lsmcontext_init(&context, secctx, secctx_len, 0);
> + security_release_secctx(&context);
> }
> audit_log_format(audit_buf, " res=%u", entry != NULL ? 1 : 0);
> audit_log_end(audit_buf);
> @@ -553,6 +559,7 @@ static int netlbl_unlhsh_remove_addr6(struct net *net,
> struct netlbl_unlhsh_addr6 *entry;
> struct audit_buffer *audit_buf;
> struct net_device *dev;
> + struct lsmcontext context;
> char *secctx;
> u32 secctx_len;
> struct lsmblob blob;
> @@ -580,7 +587,8 @@ static int netlbl_unlhsh_remove_addr6(struct net *net,
> security_secid_to_secctx(&blob,
> &secctx, &secctx_len) == 0) {
> audit_log_format(audit_buf, " sec_obj=%s", secctx);
> - security_release_secctx(secctx, secctx_len);
> + lsmcontext_init(&context, secctx, secctx_len, 0);
> + security_release_secctx(&context);
> }
> audit_log_format(audit_buf, " res=%u", entry != NULL ? 1 : 0);
> audit_log_end(audit_buf);
> @@ -1094,6 +1102,7 @@ static int netlbl_unlabel_staticlist_gen(u32 cmd,
> int ret_val = -ENOMEM;
> struct netlbl_unlhsh_walk_arg *cb_arg = arg;
> struct net_device *dev;
> + struct lsmcontext context;
> void *data;
> u32 secid;
> char *secctx;
> @@ -1161,7 +1170,9 @@ static int netlbl_unlabel_staticlist_gen(u32 cmd,
> NLBL_UNLABEL_A_SECCTX,
> secctx_len,
> secctx);
> - security_release_secctx(secctx, secctx_len);
> + /* scaffolding */
> + lsmcontext_init(&context, secctx, secctx_len, 0);
> + security_release_secctx(&context);
> if (ret_val != 0)
> goto list_cb_failure;
>
> diff --git a/net/netlabel/netlabel_user.c b/net/netlabel/netlabel_user.c
> index 2ccc6567e2a2..94aea4985b74 100644
> --- a/net/netlabel/netlabel_user.c
> +++ b/net/netlabel/netlabel_user.c
> @@ -98,6 +98,7 @@ struct audit_buffer *netlbl_audit_start_common(int type,
> struct netlbl_audit *audit_info)
> {
> struct audit_buffer *audit_buf;
> + struct lsmcontext context;
> char *secctx;
> u32 secctx_len;
> struct lsmblob blob;
> @@ -117,7 +118,8 @@ struct audit_buffer *netlbl_audit_start_common(int type,
> if (audit_info->secid != 0 &&
> security_secid_to_secctx(&blob, &secctx, &secctx_len) == 0) {
> audit_log_format(audit_buf, " subj=%s", secctx);
> - security_release_secctx(secctx, secctx_len);
> + lsmcontext_init(&context, secctx, secctx_len, 0);/*scaffolding*/
> + security_release_secctx(&context);
> }
>
> return audit_buf;
> diff --git a/security/security.c b/security/security.c
> index 82e29c477fa4..3563b1e2f8f9 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -2116,17 +2116,19 @@ int security_secctx_to_secid(const char *secdata, u32 seclen,
> }
> EXPORT_SYMBOL(security_secctx_to_secid);
>
> -void security_release_secctx(char *secdata, u32 seclen)
> +void security_release_secctx(struct lsmcontext *cp)
> {
> struct security_hook_list *hp;
> - int *display = current->security;
>
> hlist_for_each_entry(hp, &security_hook_heads.release_secctx, list)
> - if (*display == LSMBLOB_INVALID ||
> - *display == hp->lsmid->slot) {
> - hp->hook.release_secctx(secdata, seclen);
> + if (cp->slot == hp->lsmid->slot) {
> + hp->hook.release_secctx(cp->context, cp->len);
> + memset(cp, 0, sizeof(*cp));
> return;
> }
> +
> + pr_warn("%s context \"%s\" from slot %d not released\n", __func__,
> + cp->context, cp->slot);
> }
> EXPORT_SYMBOL(security_release_secctx);
>
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index ad646b865295..3d571c438dfa 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -4439,11 +4439,16 @@ static int smack_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid)
> return 0;
> }
>
> -/*
> - * There used to be a smack_release_secctx hook
> - * that did nothing back when hooks were in a vector.
> - * Now that there's a list such a hook adds cost.
> +/**
> + * smack_release_secctx - do everything necessary to free a context
> + * @secdata: Unused
> + * @seclen: Unused
> + *
> + * Do nothing but hold a slot in the hooks list.
> */
> +static void smack_release_secctx(char *secdata, u32 seclen)
> +{
> +}
>
> static int smack_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen)
> {
> @@ -4685,6 +4690,7 @@ static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
> LSM_HOOK_INIT(ismaclabel, smack_ismaclabel),
> LSM_HOOK_INIT(secid_to_secctx, smack_secid_to_secctx),
> LSM_HOOK_INIT(secctx_to_secid, smack_secctx_to_secid),
> + LSM_HOOK_INIT(release_secctx, smack_release_secctx),
> LSM_HOOK_INIT(inode_notifysecctx, smack_inode_notifysecctx),
> LSM_HOOK_INIT(inode_setsecctx, smack_inode_setsecctx),
> LSM_HOOK_INIT(inode_getsecctx, smack_inode_getsecctx),
>
^ permalink raw reply
* Re: [PATCH v4 17/23] LSM: Use lsmcontext in security_secid_to_secctx
From: John Johansen @ 2019-06-27 21:34 UTC (permalink / raw)
To: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
selinux
Cc: keescook, penguin-kernel, paul, sds
In-Reply-To: <20190626192234.11725-18-casey@schaufler-ca.com>
On 6/26/19 12:22 PM, Casey Schaufler wrote:
> Replace the (secctx,seclen) pointer pair with a single
> lsmcontext pointer to allow return of the LSM identifier
> along with the context and context length. This allows
> security_release_secctx() to know how to release the
> context. Callers have been modified to use or save the
> returned data from the new structure.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Reviewed-by: John Johansen <john.johansen@canonical.com>
> ---
> drivers/android/binder.c | 24 ++++++---------
> include/linux/security.h | 4 +--
> include/net/scm.h | 9 ++----
> kernel/audit.c | 29 +++++++-----------
> kernel/auditsc.c | 31 +++++++------------
> net/ipv4/ip_sockglue.c | 7 ++---
> net/netfilter/nf_conntrack_netlink.c | 14 +++++----
> net/netfilter/nf_conntrack_standalone.c | 7 ++---
> net/netfilter/nfnetlink_queue.c | 5 +++-
> net/netlabel/netlabel_unlabeled.c | 40 ++++++++-----------------
> net/netlabel/netlabel_user.c | 7 ++---
> security/security.c | 9 ++++--
> 12 files changed, 72 insertions(+), 114 deletions(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index 89e574be34cc..5d417a7b9bb3 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -2874,9 +2874,7 @@ static void binder_transaction(struct binder_proc *proc,
> binder_size_t last_fixup_min_off = 0;
> struct binder_context *context = proc->context;
> int t_debug_id = atomic_inc_return(&binder_last_id);
> - char *secctx = NULL;
> - u32 secctx_sz = 0;
> - struct lsmcontext scaff; /* scaffolding */
> + struct lsmcontext lsmctx;
>
> e = binder_transaction_log_add(&binder_transaction_log);
> e->debug_id = t_debug_id;
> @@ -3123,14 +3121,14 @@ static void binder_transaction(struct binder_proc *proc,
> struct lsmblob blob;
>
> security_task_getsecid(proc->tsk, &blob);
> - ret = security_secid_to_secctx(&blob, &secctx, &secctx_sz);
> + ret = security_secid_to_secctx(&blob, &lsmctx);
> if (ret) {
> return_error = BR_FAILED_REPLY;
> return_error_param = ret;
> return_error_line = __LINE__;
> goto err_get_secctx_failed;
> }
> - extra_buffers_size += ALIGN(secctx_sz, sizeof(u64));
> + extra_buffers_size += ALIGN(lsmctx.len, sizeof(u64));
> }
>
> trace_binder_transaction(reply, t, target_node);
> @@ -3149,19 +3147,17 @@ static void binder_transaction(struct binder_proc *proc,
> t->buffer = NULL;
> goto err_binder_alloc_buf_failed;
> }
> - if (secctx) {
> + if (lsmctx.context) {
> size_t buf_offset = ALIGN(tr->data_size, sizeof(void *)) +
> ALIGN(tr->offsets_size, sizeof(void *)) +
> ALIGN(extra_buffers_size, sizeof(void *)) -
> - ALIGN(secctx_sz, sizeof(u64));
> + ALIGN(lsmctx.len, sizeof(u64));
>
> t->security_ctx = (uintptr_t)t->buffer->user_data + buf_offset;
> binder_alloc_copy_to_buffer(&target_proc->alloc,
> t->buffer, buf_offset,
> - secctx, secctx_sz);
> - lsmcontext_init(&scaff, secctx, secctx_sz, 0);
> - security_release_secctx(&scaff);
> - secctx = NULL;
> + lsmctx.context, lsmctx.len);
> + security_release_secctx(&lsmctx);
> }
> t->buffer->debug_id = t->debug_id;
> t->buffer->transaction = t;
> @@ -3481,10 +3477,8 @@ static void binder_transaction(struct binder_proc *proc,
> t->buffer->transaction = NULL;
> binder_alloc_free_buf(&target_proc->alloc, t->buffer);
> err_binder_alloc_buf_failed:
> - if (secctx) {
> - lsmcontext_init(&scaff, secctx, secctx_sz, 0);
> - security_release_secctx(&scaff);
> - }
> + if (lsmctx.context)
> + security_release_secctx(&lsmctx);
> err_get_secctx_failed:
> kfree(tcomplete);
> binder_stats_deleted(BINDER_STAT_TRANSACTION_COMPLETE);
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 046012a7255f..7255825aa697 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -481,7 +481,7 @@ int security_setprocattr(const char *lsm, const char *name, void *value,
> size_t size);
> int security_netlink_send(struct sock *sk, struct sk_buff *skb);
> int security_ismaclabel(const char *name);
> -int security_secid_to_secctx(struct lsmblob *blob, char **secdata, u32 *seclen);
> +int security_secid_to_secctx(struct lsmblob *blob, struct lsmcontext *cp);
> int security_secctx_to_secid(const char *secdata, u32 seclen,
> struct lsmblob *blob);
> void security_release_secctx(struct lsmcontext *cp);
> @@ -1263,7 +1263,7 @@ static inline int security_ismaclabel(const char *name)
> }
>
> static inline int security_secid_to_secctx(struct lsmblob *blob,
> - char **secdata, u32 *seclen)
> + struct lsmcontext *cp)
> {
> return -EOPNOTSUPP;
> }
> diff --git a/include/net/scm.h b/include/net/scm.h
> index 6c7c3c229e4a..4a6ad8caf423 100644
> --- a/include/net/scm.h
> +++ b/include/net/scm.h
> @@ -93,17 +93,14 @@ static __inline__ int scm_send(struct socket *sock, struct msghdr *msg,
> static inline void scm_passec(struct socket *sock, struct msghdr *msg, struct scm_cookie *scm)
> {
> struct lsmcontext context;
> - char *secdata;
> - u32 seclen;
> int err;
>
> if (test_bit(SOCK_PASSSEC, &sock->flags)) {
> - err = security_secid_to_secctx(&scm->lsmblob, &secdata,
> - &seclen);
> + err = security_secid_to_secctx(&scm->lsmblob, &context);
>
> if (!err) {
> - put_cmsg(msg, SOL_SOCKET, SCM_SECURITY, seclen, secdata);
> - lsmcontext_init(&context, secdata, seclen, 0);/*scaffolding*/
> + put_cmsg(msg, SOL_SOCKET, SCM_SECURITY,
> + context.len, context.context);
> security_release_secctx(&context);
> }
> }
> diff --git a/kernel/audit.c b/kernel/audit.c
> index f844a2a642e6..436c23429319 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1191,9 +1191,8 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
> struct audit_buffer *ab;
> u16 msg_type = nlh->nlmsg_type;
> struct audit_sig_info *sig_data;
> - char *ctx = NULL;
> u32 len;
> - struct lsmcontext scaff; /* scaffolding */
> + struct lsmcontext context;
>
> err = audit_netlink_ok(skb, msg_type);
> if (err)
> @@ -1431,25 +1430,22 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
> case AUDIT_SIGNAL_INFO:
> len = 0;
> if (lsmblob_is_set(&audit_sig_lsm)) {
> - err = security_secid_to_secctx(&audit_sig_lsm, &ctx,
> - &len);
> + err = security_secid_to_secctx(&audit_sig_lsm,
> + &context);
> if (err)
> return err;
> }
> sig_data = kmalloc(sizeof(*sig_data) + len, GFP_KERNEL);
> if (!sig_data) {
> - if (lsmblob_is_set(&audit_sig_lsm)) {
> - lsmcontext_init(&scaff, ctx, len, 0);
> - security_release_secctx(&scaff);
> - }
> + if (lsmblob_is_set(&audit_sig_lsm))
> + security_release_secctx(&context);
> return -ENOMEM;
> }
> sig_data->uid = from_kuid(&init_user_ns, audit_sig_uid);
> sig_data->pid = audit_sig_pid;
> if (lsmblob_is_set(&audit_sig_lsm)) {
> - memcpy(sig_data->ctx, ctx, len);
> - lsmcontext_init(&scaff, ctx, len, 0);
> - security_release_secctx(&scaff);
> + memcpy(sig_data->ctx, context.context, context.len);
> + security_release_secctx(&context);
> }
> audit_send_reply(skb, seq, AUDIT_SIGNAL_INFO, 0, 0,
> sig_data, sizeof(*sig_data) + len);
> @@ -2074,26 +2070,23 @@ void audit_log_key(struct audit_buffer *ab, char *key)
>
> int audit_log_task_context(struct audit_buffer *ab)
> {
> - char *ctx = NULL;
> - unsigned len;
> int error;
> struct lsmblob blob;
> - struct lsmcontext scaff; /* scaffolding */
> + struct lsmcontext context;
>
> security_task_getsecid(current, &blob);
> if (!lsmblob_is_set(&blob))
> return 0;
>
> - error = security_secid_to_secctx(&blob, &ctx, &len);
> + error = security_secid_to_secctx(&blob, &context);
> if (error) {
> if (error != -EINVAL)
> goto error_path;
> return 0;
> }
>
> - audit_log_format(ab, " subj=%s", ctx);
> - lsmcontext_init(&scaff, ctx, len, 0);
> - security_release_secctx(&scaff);
> + audit_log_format(ab, " subj=%s", context.context);
> + security_release_secctx(&context);
> return 0;
>
> error_path:
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 9fab0e7d90c3..0478680cd0a8 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -943,9 +943,7 @@ static int audit_log_pid_context(struct audit_context *context, pid_t pid,
> struct lsmblob *blob, char *comm)
> {
> struct audit_buffer *ab;
> - struct lsmcontext lsmcxt;
> - char *ctx = NULL;
> - u32 len;
> + struct lsmcontext lsmctx;
> int rc = 0;
>
> ab = audit_log_start(context, GFP_KERNEL, AUDIT_OBJ_PID);
> @@ -956,13 +954,12 @@ static int audit_log_pid_context(struct audit_context *context, pid_t pid,
> from_kuid(&init_user_ns, auid),
> from_kuid(&init_user_ns, uid), sessionid);
> if (lsmblob_is_set(blob)) {
> - if (security_secid_to_secctx(blob, &ctx, &len)) {
> + if (security_secid_to_secctx(blob, &lsmctx)) {
> audit_log_format(ab, " obj=(none)");
> rc = 1;
> } else {
> - audit_log_format(ab, " obj=%s", ctx);
> - lsmcontext_init(&lsmcxt, ctx, len, 0); /*scaffolding*/
> - security_release_secctx(&lsmcxt);
> + audit_log_format(ab, " obj=%s", lsmctx.context);
> + security_release_secctx(&lsmctx);
> }
> }
> audit_log_format(ab, " ocomm=");
> @@ -1174,7 +1171,6 @@ static void audit_log_fcaps(struct audit_buffer *ab, struct audit_names *name)
>
> static void show_special(struct audit_context *context, int *call_panic)
> {
> - struct lsmcontext lsmcxt;
> struct audit_buffer *ab;
> int i;
>
> @@ -1198,17 +1194,15 @@ static void show_special(struct audit_context *context, int *call_panic)
> from_kgid(&init_user_ns, context->ipc.gid),
> context->ipc.mode);
> if (osid) {
> - char *ctx = NULL;
> - u32 len;
> + struct lsmcontext lsmcxt;
> struct lsmblob blob;
>
> lsmblob_init(&blob, osid);
> - if (security_secid_to_secctx(&blob, &ctx, &len)) {
> + if (security_secid_to_secctx(&blob, &lsmcxt)) {
> audit_log_format(ab, " osid=%u", osid);
> *call_panic = 1;
> } else {
> - audit_log_format(ab, " obj=%s", ctx);
> - lsmcontext_init(&lsmcxt, ctx, len, 0);
> + audit_log_format(ab, " obj=%s", lsmcxt.context);
> security_release_secctx(&lsmcxt);
> }
> }
> @@ -1353,20 +1347,17 @@ static void audit_log_name(struct audit_context *context, struct audit_names *n,
> MAJOR(n->rdev),
> MINOR(n->rdev));
> if (n->osid != 0) {
> - char *ctx = NULL;
> - u32 len;
> struct lsmblob blob;
> - struct lsmcontext lsmcxt;
> + struct lsmcontext lsmctx;
>
> lsmblob_init(&blob, n->osid);
> - if (security_secid_to_secctx(&blob, &ctx, &len)) {
> + if (security_secid_to_secctx(&blob, &lsmctx)) {
> audit_log_format(ab, " osid=%u", n->osid);
> if (call_panic)
> *call_panic = 2;
> } else {
> - audit_log_format(ab, " obj=%s", ctx);
> - lsmcontext_init(&lsmcxt, ctx, len, 0); /* scaffolding */
> - security_release_secctx(&lsmcxt);
> + audit_log_format(ab, " obj=%s", lsmctx.context);
> + security_release_secctx(&lsmctx);
> }
> }
>
> diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
> index 7834c357b60b..80ae0c5a1301 100644
> --- a/net/ipv4/ip_sockglue.c
> +++ b/net/ipv4/ip_sockglue.c
> @@ -132,20 +132,17 @@ static void ip_cmsg_recv_security(struct msghdr *msg, struct sk_buff *skb)
> {
> struct lsmcontext context;
> struct lsmblob lb;
> - char *secdata;
> - u32 seclen;
> int err;
>
> err = security_socket_getpeersec_dgram(NULL, skb, &lb);
> if (err)
> return;
>
> - err = security_secid_to_secctx(&lb, &secdata, &seclen);
> + err = security_secid_to_secctx(&lb, &context);
> if (err)
> return;
>
> - put_cmsg(msg, SOL_IP, SCM_SECURITY, seclen, secdata);
> - lsmcontext_init(&context, secdata, seclen, 0); /* scaffolding */
> + put_cmsg(msg, SOL_IP, SCM_SECURITY, context.len, context.context);
> security_release_secctx(&context);
> }
>
> diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
> index 6954e6600583..403307ff0fff 100644
> --- a/net/netfilter/nf_conntrack_netlink.c
> +++ b/net/netfilter/nf_conntrack_netlink.c
> @@ -328,13 +328,12 @@ static int ctnetlink_dump_mark(struct sk_buff *skb, const struct nf_conn *ct)
> static int ctnetlink_dump_secctx(struct sk_buff *skb, const struct nf_conn *ct)
> {
> struct nlattr *nest_secctx;
> - int len, ret;
> - char *secctx;
> + int ret;
> struct lsmblob blob;
> struct lsmcontext context;
>
> lsmblob_init(&blob, ct->secmark);
> - ret = security_secid_to_secctx(&blob, &secctx, &len);
> + ret = security_secid_to_secctx(&blob, &context);
> if (ret)
> return 0;
>
> @@ -343,13 +342,12 @@ static int ctnetlink_dump_secctx(struct sk_buff *skb, const struct nf_conn *ct)
> if (!nest_secctx)
> goto nla_put_failure;
>
> - if (nla_put_string(skb, CTA_SECCTX_NAME, secctx))
> + if (nla_put_string(skb, CTA_SECCTX_NAME, context.context))
> goto nla_put_failure;
> nla_nest_end(skb, nest_secctx);
>
> ret = 0;
> nla_put_failure:
> - lsmcontext_init(&context, secctx, len, 0); /* scaffolding */
> security_release_secctx(&context);
> return ret;
> }
> @@ -620,12 +618,16 @@ static inline int ctnetlink_secctx_size(const struct nf_conn *ct)
> #ifdef CONFIG_NF_CONNTRACK_SECMARK
> int len, ret;
> struct lsmblob blob;
> + struct lsmcontext context;
>
> lsmblob_init(&blob, ct->secmark);
> - ret = security_secid_to_secctx(&blob, NULL, &len);
> + ret = security_secid_to_secctx(&blob, &context);
> if (ret)
> return 0;
>
> + len = context.len;
> + security_release_secctx(&context);
> +
> return nla_total_size(0) /* CTA_SECCTX */
> + nla_total_size(sizeof(char) * len); /* CTA_SECCTX_NAME */
> #else
> diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
> index 79158ad0486e..fcb51ab2bb8b 100644
> --- a/net/netfilter/nf_conntrack_standalone.c
> +++ b/net/netfilter/nf_conntrack_standalone.c
> @@ -173,19 +173,16 @@ static void ct_seq_stop(struct seq_file *s, void *v)
> static void ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
> {
> int ret;
> - u32 len;
> - char *secctx;
> struct lsmblob blob;
> struct lsmcontext context;
>
> lsmblob_init(&blob, ct->secmark);
> - ret = security_secid_to_secctx(&blob, &secctx, &len);
> + ret = security_secid_to_secctx(&blob, &context);
> if (ret)
> return;
>
> - seq_printf(s, "secctx=%s ", secctx);
> + seq_printf(s, "secctx=%s ", context.context);
>
> - lsmcontext_init(&context, secctx, len, 0); /* scaffolding */
> security_release_secctx(&context);
> }
> #else
> diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
> index fe8403ef4e89..6da00c7add5b 100644
> --- a/net/netfilter/nfnetlink_queue.c
> +++ b/net/netfilter/nfnetlink_queue.c
> @@ -310,6 +310,7 @@ static u32 nfqnl_get_sk_secctx(struct sk_buff *skb, char **secdata)
> u32 seclen = 0;
> #if IS_ENABLED(CONFIG_NETWORK_SECMARK)
> struct lsmblob blob;
> + struct lsmcontext context;
>
> if (!skb || !sk_fullsock(skb->sk))
> return 0;
> @@ -318,10 +319,12 @@ static u32 nfqnl_get_sk_secctx(struct sk_buff *skb, char **secdata)
>
> if (skb->secmark) {
> lsmblob_init(&blob, skb->secmark);
> - security_secid_to_secctx(&blob, secdata, &seclen);
> + security_secid_to_secctx(&blob, &context);
> + *secdata = context.context;
> }
>
> read_unlock_bh(&skb->sk->sk_callback_lock);
> + seclen = context.len;
> #endif
> return seclen;
> }
> diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
> index 15b1945853be..4716e0011ba5 100644
> --- a/net/netlabel/netlabel_unlabeled.c
> +++ b/net/netlabel/netlabel_unlabeled.c
> @@ -388,8 +388,6 @@ int netlbl_unlhsh_add(struct net *net,
> struct netlbl_unlhsh_iface *iface;
> struct audit_buffer *audit_buf = NULL;
> struct lsmcontext context;
> - char *secctx = NULL;
> - u32 secctx_len;
> struct lsmblob blob;
>
> if (addr_len != sizeof(struct in_addr) &&
> @@ -454,12 +452,9 @@ int netlbl_unlhsh_add(struct net *net,
> rcu_read_unlock();
> if (audit_buf != NULL) {
> lsmblob_init(&blob, secid);
> - if (security_secid_to_secctx(&blob,
> - &secctx,
> - &secctx_len) == 0) {
> - audit_log_format(audit_buf, " sec_obj=%s", secctx);
> - /* scaffolding */
> - lsmcontext_init(&context, secctx, secctx_len, 0);
> + if (security_secid_to_secctx(&blob, &context) == 0) {
> + audit_log_format(audit_buf, " sec_obj=%s",
> + context.context);
> security_release_secctx(&context);
> }
> audit_log_format(audit_buf, " res=%u", ret_val == 0 ? 1 : 0);
> @@ -492,8 +487,6 @@ static int netlbl_unlhsh_remove_addr4(struct net *net,
> struct audit_buffer *audit_buf;
> struct net_device *dev;
> struct lsmcontext context;
> - char *secctx;
> - u32 secctx_len;
> struct lsmblob blob;
>
> spin_lock(&netlbl_unlhsh_lock);
> @@ -517,11 +510,9 @@ static int netlbl_unlhsh_remove_addr4(struct net *net,
> if (entry != NULL)
> lsmblob_init(&blob, entry->secid);
> if (entry != NULL &&
> - security_secid_to_secctx(&blob,
> - &secctx, &secctx_len) == 0) {
> - audit_log_format(audit_buf, " sec_obj=%s", secctx);
> - /* scaffolding */
> - lsmcontext_init(&context, secctx, secctx_len, 0);
> + security_secid_to_secctx(&blob, &context) == 0) {
> + audit_log_format(audit_buf, " sec_obj=%s",
> + context.context);
> security_release_secctx(&context);
> }
> audit_log_format(audit_buf, " res=%u", entry != NULL ? 1 : 0);
> @@ -560,8 +551,6 @@ static int netlbl_unlhsh_remove_addr6(struct net *net,
> struct audit_buffer *audit_buf;
> struct net_device *dev;
> struct lsmcontext context;
> - char *secctx;
> - u32 secctx_len;
> struct lsmblob blob;
>
> spin_lock(&netlbl_unlhsh_lock);
> @@ -584,10 +573,9 @@ static int netlbl_unlhsh_remove_addr6(struct net *net,
> if (entry != NULL)
> lsmblob_init(&blob, entry->secid);
> if (entry != NULL &&
> - security_secid_to_secctx(&blob,
> - &secctx, &secctx_len) == 0) {
> - audit_log_format(audit_buf, " sec_obj=%s", secctx);
> - lsmcontext_init(&context, secctx, secctx_len, 0);
> + security_secid_to_secctx(&blob, &context) == 0) {
> + audit_log_format(audit_buf, " sec_obj=%s",
> + context.context);
> security_release_secctx(&context);
> }
> audit_log_format(audit_buf, " res=%u", entry != NULL ? 1 : 0);
> @@ -1105,8 +1093,6 @@ static int netlbl_unlabel_staticlist_gen(u32 cmd,
> struct lsmcontext context;
> void *data;
> u32 secid;
> - char *secctx;
> - u32 secctx_len;
> struct lsmblob blob;
>
> data = genlmsg_put(cb_arg->skb, NETLINK_CB(cb_arg->nl_cb->skb).portid,
> @@ -1163,15 +1149,13 @@ static int netlbl_unlabel_staticlist_gen(u32 cmd,
> }
>
> lsmblob_init(&blob, secid);
> - ret_val = security_secid_to_secctx(&blob, &secctx, &secctx_len);
> + ret_val = security_secid_to_secctx(&blob, &context);
> if (ret_val != 0)
> goto list_cb_failure;
> ret_val = nla_put(cb_arg->skb,
> NLBL_UNLABEL_A_SECCTX,
> - secctx_len,
> - secctx);
> - /* scaffolding */
> - lsmcontext_init(&context, secctx, secctx_len, 0);
> + context.len,
> + context.context);
> security_release_secctx(&context);
> if (ret_val != 0)
> goto list_cb_failure;
> diff --git a/net/netlabel/netlabel_user.c b/net/netlabel/netlabel_user.c
> index 94aea4985b74..2d1307f65250 100644
> --- a/net/netlabel/netlabel_user.c
> +++ b/net/netlabel/netlabel_user.c
> @@ -99,8 +99,6 @@ struct audit_buffer *netlbl_audit_start_common(int type,
> {
> struct audit_buffer *audit_buf;
> struct lsmcontext context;
> - char *secctx;
> - u32 secctx_len;
> struct lsmblob blob;
>
> if (audit_enabled == AUDIT_OFF)
> @@ -116,9 +114,8 @@ struct audit_buffer *netlbl_audit_start_common(int type,
>
> lsmblob_init(&blob, audit_info->secid);
> if (audit_info->secid != 0 &&
> - security_secid_to_secctx(&blob, &secctx, &secctx_len) == 0) {
> - audit_log_format(audit_buf, " subj=%s", secctx);
> - lsmcontext_init(&context, secctx, secctx_len, 0);/*scaffolding*/
> + security_secid_to_secctx(&blob, &context) == 0) {
> + audit_log_format(audit_buf, " subj=%s", context.context);
> security_release_secctx(&context);
> }
>
> diff --git a/security/security.c b/security/security.c
> index 3563b1e2f8f9..97b468f6e6a9 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -2081,7 +2081,7 @@ int security_ismaclabel(const char *name)
> }
> EXPORT_SYMBOL(security_ismaclabel);
>
> -int security_secid_to_secctx(struct lsmblob *blob, char **secdata, u32 *seclen)
> +int security_secid_to_secctx(struct lsmblob *blob, struct lsmcontext *cp)
> {
> struct security_hook_list *hp;
> int *display = current->security;
> @@ -2089,10 +2089,13 @@ int security_secid_to_secctx(struct lsmblob *blob, char **secdata, u32 *seclen)
> hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list) {
> if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
> continue;
> - if (*display == LSMBLOB_INVALID || *display == hp->lsmid->slot)
> + if (*display == LSMBLOB_INVALID ||
> + *display == hp->lsmid->slot) {
> + cp->slot = hp->lsmid->slot;
> return hp->hook.secid_to_secctx(
> blob->secid[hp->lsmid->slot],
> - secdata, seclen);
> + &cp->context, &cp->len);
> + }
> }
> return 0;
> }
>
^ permalink raw reply
* Re: [PATCH v4 18/23] LSM: Use lsmcontext in security_dentry_init_security
From: John Johansen @ 2019-06-27 21:34 UTC (permalink / raw)
To: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
selinux
Cc: keescook, penguin-kernel, paul, sds
In-Reply-To: <20190626192234.11725-19-casey@schaufler-ca.com>
On 6/26/19 12:22 PM, Casey Schaufler wrote:
> Change the security_dentry_init_security() interface to
> fill an lsmcontext structure instead of a void * data area
> and a length. The lone caller of this interface is NFS4,
> which may make copies of the data using its own mechanisms.
> A rework of the nfs4 code to use the lsmcontext properly
> is a significant project, so the coward's way out is taken,
> and the lsmcontext data from security_dentry_init_security()
> is copied, then released directly.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Reviewed-by: John Johansen <john.johansen@canonical.com>
> ---
> fs/nfs/nfs4proc.c | 26 ++++++++++++++++----------
> include/linux/security.h | 7 +++----
> security/security.c | 19 +++++++++++++++----
> 3 files changed, 34 insertions(+), 18 deletions(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index af1c0db29c39..952f805965bb 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -113,6 +113,7 @@ static inline struct nfs4_label *
> nfs4_label_init_security(struct inode *dir, struct dentry *dentry,
> struct iattr *sattr, struct nfs4_label *label)
> {
> + struct lsmcontext context;
> int err;
>
> if (label == NULL)
> @@ -122,21 +123,26 @@ nfs4_label_init_security(struct inode *dir, struct dentry *dentry,
> return NULL;
>
> err = security_dentry_init_security(dentry, sattr->ia_mode,
> - &dentry->d_name, (void **)&label->label, &label->len);
> - if (err == 0)
> - return label;
> + &dentry->d_name, &context);
> +
> + if (err)
> + return NULL;
> +
> + label->label = kmemdup(context.context, context.len, GFP_KERNEL);
> + if (label->label == NULL)
> + label = NULL;
> + else
> + label->len = context.len;
> +
> + security_release_secctx(&context);
> +
> + return label;
>
> - return NULL;
> }
> static inline void
> nfs4_label_release_security(struct nfs4_label *label)
> {
> - struct lsmcontext scaff; /* scaffolding */
> -
> - if (label) {
> - lsmcontext_init(&scaff, label->label, label->len, 0);
> - security_release_secctx(&scaff);
> - }
> + kfree(label->label);
> }
> static inline u32 *nfs4_bitmask(struct nfs_server *server, struct nfs4_label *label)
> {
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 7255825aa697..2674eb70c2d7 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -352,8 +352,8 @@ int security_sb_clone_mnt_opts(const struct super_block *oldsb,
> int security_add_mnt_opt(const char *option, const char *val,
> int len, void **mnt_opts);
> int security_dentry_init_security(struct dentry *dentry, int mode,
> - const struct qstr *name, void **ctx,
> - u32 *ctxlen);
> + const struct qstr *name,
> + struct lsmcontext *ctx);
> int security_dentry_create_files_as(struct dentry *dentry, int mode,
> struct qstr *name,
> const struct cred *old,
> @@ -724,8 +724,7 @@ static inline void security_inode_free(struct inode *inode)
> static inline int security_dentry_init_security(struct dentry *dentry,
> int mode,
> const struct qstr *name,
> - void **ctx,
> - u32 *ctxlen)
> + struct lsmcontext *ctx)
> {
> return -EOPNOTSUPP;
> }
> diff --git a/security/security.c b/security/security.c
> index 97b468f6e6a9..61cdc6bcd32e 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1024,11 +1024,22 @@ void security_inode_free(struct inode *inode)
> }
>
> int security_dentry_init_security(struct dentry *dentry, int mode,
> - const struct qstr *name, void **ctx,
> - u32 *ctxlen)
> + const struct qstr *name,
> + struct lsmcontext *cp)
> {
> - return call_int_hook(dentry_init_security, -EOPNOTSUPP, dentry, mode,
> - name, ctx, ctxlen);
> + int *display = current->security;
> + struct security_hook_list *hp;
> +
> + hlist_for_each_entry(hp, &security_hook_heads.dentry_init_security,
> + list)
> + if (*display == LSMBLOB_INVALID ||
> + *display == hp->lsmid->slot) {
> + cp->slot = hp->lsmid->slot;
> + return hp->hook.dentry_init_security(dentry, mode,
> + name, (void **)&cp->context, &cp->len);
> + }
> +
> + return -EOPNOTSUPP;
> }
> EXPORT_SYMBOL(security_dentry_init_security);
>
>
^ permalink raw reply
* Re: [PATCH v4 17/23] LSM: Use lsmcontext in security_secid_to_secctx
From: John Johansen @ 2019-06-27 21:36 UTC (permalink / raw)
To: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
selinux
Cc: keescook, penguin-kernel, paul, sds
In-Reply-To: <3f251d4b-0258-e011-8ff4-f0a23f4b4e4c@canonical.com>
On 6/27/19 2:34 PM, John Johansen wrote:
> On 6/26/19 12:22 PM, Casey Schaufler wrote:
>> Replace the (secctx,seclen) pointer pair with a single
>> lsmcontext pointer to allow return of the LSM identifier
>> along with the context and context length. This allows
>> security_release_secctx() to know how to release the
>> context. Callers have been modified to use or save the
>> returned data from the new structure.
>>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>
> Reviewed-by: John Johansen <john.johansen@canonical.com>
sorry, no I hit reply on the wrong patch. I want to see the
revision of this one.
>
>
>> ---
>> drivers/android/binder.c | 24 ++++++---------
>> include/linux/security.h | 4 +--
>> include/net/scm.h | 9 ++----
>> kernel/audit.c | 29 +++++++-----------
>> kernel/auditsc.c | 31 +++++++------------
>> net/ipv4/ip_sockglue.c | 7 ++---
>> net/netfilter/nf_conntrack_netlink.c | 14 +++++----
>> net/netfilter/nf_conntrack_standalone.c | 7 ++---
>> net/netfilter/nfnetlink_queue.c | 5 +++-
>> net/netlabel/netlabel_unlabeled.c | 40 ++++++++-----------------
>> net/netlabel/netlabel_user.c | 7 ++---
>> security/security.c | 9 ++++--
>> 12 files changed, 72 insertions(+), 114 deletions(-)
>>
>> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
>> index 89e574be34cc..5d417a7b9bb3 100644
>> --- a/drivers/android/binder.c
>> +++ b/drivers/android/binder.c
>> @@ -2874,9 +2874,7 @@ static void binder_transaction(struct binder_proc *proc,
>> binder_size_t last_fixup_min_off = 0;
>> struct binder_context *context = proc->context;
>> int t_debug_id = atomic_inc_return(&binder_last_id);
>> - char *secctx = NULL;
>> - u32 secctx_sz = 0;
>> - struct lsmcontext scaff; /* scaffolding */
>> + struct lsmcontext lsmctx;
>>
>> e = binder_transaction_log_add(&binder_transaction_log);
>> e->debug_id = t_debug_id;
>> @@ -3123,14 +3121,14 @@ static void binder_transaction(struct binder_proc *proc,
>> struct lsmblob blob;
>>
>> security_task_getsecid(proc->tsk, &blob);
>> - ret = security_secid_to_secctx(&blob, &secctx, &secctx_sz);
>> + ret = security_secid_to_secctx(&blob, &lsmctx);
>> if (ret) {
>> return_error = BR_FAILED_REPLY;
>> return_error_param = ret;
>> return_error_line = __LINE__;
>> goto err_get_secctx_failed;
>> }
>> - extra_buffers_size += ALIGN(secctx_sz, sizeof(u64));
>> + extra_buffers_size += ALIGN(lsmctx.len, sizeof(u64));
>> }
>>
>> trace_binder_transaction(reply, t, target_node);
>> @@ -3149,19 +3147,17 @@ static void binder_transaction(struct binder_proc *proc,
>> t->buffer = NULL;
>> goto err_binder_alloc_buf_failed;
>> }
>> - if (secctx) {
>> + if (lsmctx.context) {
>> size_t buf_offset = ALIGN(tr->data_size, sizeof(void *)) +
>> ALIGN(tr->offsets_size, sizeof(void *)) +
>> ALIGN(extra_buffers_size, sizeof(void *)) -
>> - ALIGN(secctx_sz, sizeof(u64));
>> + ALIGN(lsmctx.len, sizeof(u64));
>>
>> t->security_ctx = (uintptr_t)t->buffer->user_data + buf_offset;
>> binder_alloc_copy_to_buffer(&target_proc->alloc,
>> t->buffer, buf_offset,
>> - secctx, secctx_sz);
>> - lsmcontext_init(&scaff, secctx, secctx_sz, 0);
>> - security_release_secctx(&scaff);
>> - secctx = NULL;
>> + lsmctx.context, lsmctx.len);
>> + security_release_secctx(&lsmctx);
>> }
>> t->buffer->debug_id = t->debug_id;
>> t->buffer->transaction = t;
>> @@ -3481,10 +3477,8 @@ static void binder_transaction(struct binder_proc *proc,
>> t->buffer->transaction = NULL;
>> binder_alloc_free_buf(&target_proc->alloc, t->buffer);
>> err_binder_alloc_buf_failed:
>> - if (secctx) {
>> - lsmcontext_init(&scaff, secctx, secctx_sz, 0);
>> - security_release_secctx(&scaff);
>> - }
>> + if (lsmctx.context)
>> + security_release_secctx(&lsmctx);
>> err_get_secctx_failed:
>> kfree(tcomplete);
>> binder_stats_deleted(BINDER_STAT_TRANSACTION_COMPLETE);
>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index 046012a7255f..7255825aa697 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -481,7 +481,7 @@ int security_setprocattr(const char *lsm, const char *name, void *value,
>> size_t size);
>> int security_netlink_send(struct sock *sk, struct sk_buff *skb);
>> int security_ismaclabel(const char *name);
>> -int security_secid_to_secctx(struct lsmblob *blob, char **secdata, u32 *seclen);
>> +int security_secid_to_secctx(struct lsmblob *blob, struct lsmcontext *cp);
>> int security_secctx_to_secid(const char *secdata, u32 seclen,
>> struct lsmblob *blob);
>> void security_release_secctx(struct lsmcontext *cp);
>> @@ -1263,7 +1263,7 @@ static inline int security_ismaclabel(const char *name)
>> }
>>
>> static inline int security_secid_to_secctx(struct lsmblob *blob,
>> - char **secdata, u32 *seclen)
>> + struct lsmcontext *cp)
>> {
>> return -EOPNOTSUPP;
>> }
>> diff --git a/include/net/scm.h b/include/net/scm.h
>> index 6c7c3c229e4a..4a6ad8caf423 100644
>> --- a/include/net/scm.h
>> +++ b/include/net/scm.h
>> @@ -93,17 +93,14 @@ static __inline__ int scm_send(struct socket *sock, struct msghdr *msg,
>> static inline void scm_passec(struct socket *sock, struct msghdr *msg, struct scm_cookie *scm)
>> {
>> struct lsmcontext context;
>> - char *secdata;
>> - u32 seclen;
>> int err;
>>
>> if (test_bit(SOCK_PASSSEC, &sock->flags)) {
>> - err = security_secid_to_secctx(&scm->lsmblob, &secdata,
>> - &seclen);
>> + err = security_secid_to_secctx(&scm->lsmblob, &context);
>>
>> if (!err) {
>> - put_cmsg(msg, SOL_SOCKET, SCM_SECURITY, seclen, secdata);
>> - lsmcontext_init(&context, secdata, seclen, 0);/*scaffolding*/
>> + put_cmsg(msg, SOL_SOCKET, SCM_SECURITY,
>> + context.len, context.context);
>> security_release_secctx(&context);
>> }
>> }
>> diff --git a/kernel/audit.c b/kernel/audit.c
>> index f844a2a642e6..436c23429319 100644
>> --- a/kernel/audit.c
>> +++ b/kernel/audit.c
>> @@ -1191,9 +1191,8 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
>> struct audit_buffer *ab;
>> u16 msg_type = nlh->nlmsg_type;
>> struct audit_sig_info *sig_data;
>> - char *ctx = NULL;
>> u32 len;
>> - struct lsmcontext scaff; /* scaffolding */
>> + struct lsmcontext context;
>>
>> err = audit_netlink_ok(skb, msg_type);
>> if (err)
>> @@ -1431,25 +1430,22 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
>> case AUDIT_SIGNAL_INFO:
>> len = 0;
>> if (lsmblob_is_set(&audit_sig_lsm)) {
>> - err = security_secid_to_secctx(&audit_sig_lsm, &ctx,
>> - &len);
>> + err = security_secid_to_secctx(&audit_sig_lsm,
>> + &context);
>> if (err)
>> return err;
>> }
>> sig_data = kmalloc(sizeof(*sig_data) + len, GFP_KERNEL);
>> if (!sig_data) {
>> - if (lsmblob_is_set(&audit_sig_lsm)) {
>> - lsmcontext_init(&scaff, ctx, len, 0);
>> - security_release_secctx(&scaff);
>> - }
>> + if (lsmblob_is_set(&audit_sig_lsm))
>> + security_release_secctx(&context);
>> return -ENOMEM;
>> }
>> sig_data->uid = from_kuid(&init_user_ns, audit_sig_uid);
>> sig_data->pid = audit_sig_pid;
>> if (lsmblob_is_set(&audit_sig_lsm)) {
>> - memcpy(sig_data->ctx, ctx, len);
>> - lsmcontext_init(&scaff, ctx, len, 0);
>> - security_release_secctx(&scaff);
>> + memcpy(sig_data->ctx, context.context, context.len);
>> + security_release_secctx(&context);
>> }
>> audit_send_reply(skb, seq, AUDIT_SIGNAL_INFO, 0, 0,
>> sig_data, sizeof(*sig_data) + len);
>> @@ -2074,26 +2070,23 @@ void audit_log_key(struct audit_buffer *ab, char *key)
>>
>> int audit_log_task_context(struct audit_buffer *ab)
>> {
>> - char *ctx = NULL;
>> - unsigned len;
>> int error;
>> struct lsmblob blob;
>> - struct lsmcontext scaff; /* scaffolding */
>> + struct lsmcontext context;
>>
>> security_task_getsecid(current, &blob);
>> if (!lsmblob_is_set(&blob))
>> return 0;
>>
>> - error = security_secid_to_secctx(&blob, &ctx, &len);
>> + error = security_secid_to_secctx(&blob, &context);
>> if (error) {
>> if (error != -EINVAL)
>> goto error_path;
>> return 0;
>> }
>>
>> - audit_log_format(ab, " subj=%s", ctx);
>> - lsmcontext_init(&scaff, ctx, len, 0);
>> - security_release_secctx(&scaff);
>> + audit_log_format(ab, " subj=%s", context.context);
>> + security_release_secctx(&context);
>> return 0;
>>
>> error_path:
>> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
>> index 9fab0e7d90c3..0478680cd0a8 100644
>> --- a/kernel/auditsc.c
>> +++ b/kernel/auditsc.c
>> @@ -943,9 +943,7 @@ static int audit_log_pid_context(struct audit_context *context, pid_t pid,
>> struct lsmblob *blob, char *comm)
>> {
>> struct audit_buffer *ab;
>> - struct lsmcontext lsmcxt;
>> - char *ctx = NULL;
>> - u32 len;
>> + struct lsmcontext lsmctx;
>> int rc = 0;
>>
>> ab = audit_log_start(context, GFP_KERNEL, AUDIT_OBJ_PID);
>> @@ -956,13 +954,12 @@ static int audit_log_pid_context(struct audit_context *context, pid_t pid,
>> from_kuid(&init_user_ns, auid),
>> from_kuid(&init_user_ns, uid), sessionid);
>> if (lsmblob_is_set(blob)) {
>> - if (security_secid_to_secctx(blob, &ctx, &len)) {
>> + if (security_secid_to_secctx(blob, &lsmctx)) {
>> audit_log_format(ab, " obj=(none)");
>> rc = 1;
>> } else {
>> - audit_log_format(ab, " obj=%s", ctx);
>> - lsmcontext_init(&lsmcxt, ctx, len, 0); /*scaffolding*/
>> - security_release_secctx(&lsmcxt);
>> + audit_log_format(ab, " obj=%s", lsmctx.context);
>> + security_release_secctx(&lsmctx);
>> }
>> }
>> audit_log_format(ab, " ocomm=");
>> @@ -1174,7 +1171,6 @@ static void audit_log_fcaps(struct audit_buffer *ab, struct audit_names *name)
>>
>> static void show_special(struct audit_context *context, int *call_panic)
>> {
>> - struct lsmcontext lsmcxt;
>> struct audit_buffer *ab;
>> int i;
>>
>> @@ -1198,17 +1194,15 @@ static void show_special(struct audit_context *context, int *call_panic)
>> from_kgid(&init_user_ns, context->ipc.gid),
>> context->ipc.mode);
>> if (osid) {
>> - char *ctx = NULL;
>> - u32 len;
>> + struct lsmcontext lsmcxt;
>> struct lsmblob blob;
>>
>> lsmblob_init(&blob, osid);
>> - if (security_secid_to_secctx(&blob, &ctx, &len)) {
>> + if (security_secid_to_secctx(&blob, &lsmcxt)) {
>> audit_log_format(ab, " osid=%u", osid);
>> *call_panic = 1;
>> } else {
>> - audit_log_format(ab, " obj=%s", ctx);
>> - lsmcontext_init(&lsmcxt, ctx, len, 0);
>> + audit_log_format(ab, " obj=%s", lsmcxt.context);
>> security_release_secctx(&lsmcxt);
>> }
>> }
>> @@ -1353,20 +1347,17 @@ static void audit_log_name(struct audit_context *context, struct audit_names *n,
>> MAJOR(n->rdev),
>> MINOR(n->rdev));
>> if (n->osid != 0) {
>> - char *ctx = NULL;
>> - u32 len;
>> struct lsmblob blob;
>> - struct lsmcontext lsmcxt;
>> + struct lsmcontext lsmctx;
>>
>> lsmblob_init(&blob, n->osid);
>> - if (security_secid_to_secctx(&blob, &ctx, &len)) {
>> + if (security_secid_to_secctx(&blob, &lsmctx)) {
>> audit_log_format(ab, " osid=%u", n->osid);
>> if (call_panic)
>> *call_panic = 2;
>> } else {
>> - audit_log_format(ab, " obj=%s", ctx);
>> - lsmcontext_init(&lsmcxt, ctx, len, 0); /* scaffolding */
>> - security_release_secctx(&lsmcxt);
>> + audit_log_format(ab, " obj=%s", lsmctx.context);
>> + security_release_secctx(&lsmctx);
>> }
>> }
>>
>> diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
>> index 7834c357b60b..80ae0c5a1301 100644
>> --- a/net/ipv4/ip_sockglue.c
>> +++ b/net/ipv4/ip_sockglue.c
>> @@ -132,20 +132,17 @@ static void ip_cmsg_recv_security(struct msghdr *msg, struct sk_buff *skb)
>> {
>> struct lsmcontext context;
>> struct lsmblob lb;
>> - char *secdata;
>> - u32 seclen;
>> int err;
>>
>> err = security_socket_getpeersec_dgram(NULL, skb, &lb);
>> if (err)
>> return;
>>
>> - err = security_secid_to_secctx(&lb, &secdata, &seclen);
>> + err = security_secid_to_secctx(&lb, &context);
>> if (err)
>> return;
>>
>> - put_cmsg(msg, SOL_IP, SCM_SECURITY, seclen, secdata);
>> - lsmcontext_init(&context, secdata, seclen, 0); /* scaffolding */
>> + put_cmsg(msg, SOL_IP, SCM_SECURITY, context.len, context.context);
>> security_release_secctx(&context);
>> }
>>
>> diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
>> index 6954e6600583..403307ff0fff 100644
>> --- a/net/netfilter/nf_conntrack_netlink.c
>> +++ b/net/netfilter/nf_conntrack_netlink.c
>> @@ -328,13 +328,12 @@ static int ctnetlink_dump_mark(struct sk_buff *skb, const struct nf_conn *ct)
>> static int ctnetlink_dump_secctx(struct sk_buff *skb, const struct nf_conn *ct)
>> {
>> struct nlattr *nest_secctx;
>> - int len, ret;
>> - char *secctx;
>> + int ret;
>> struct lsmblob blob;
>> struct lsmcontext context;
>>
>> lsmblob_init(&blob, ct->secmark);
>> - ret = security_secid_to_secctx(&blob, &secctx, &len);
>> + ret = security_secid_to_secctx(&blob, &context);
>> if (ret)
>> return 0;
>>
>> @@ -343,13 +342,12 @@ static int ctnetlink_dump_secctx(struct sk_buff *skb, const struct nf_conn *ct)
>> if (!nest_secctx)
>> goto nla_put_failure;
>>
>> - if (nla_put_string(skb, CTA_SECCTX_NAME, secctx))
>> + if (nla_put_string(skb, CTA_SECCTX_NAME, context.context))
>> goto nla_put_failure;
>> nla_nest_end(skb, nest_secctx);
>>
>> ret = 0;
>> nla_put_failure:
>> - lsmcontext_init(&context, secctx, len, 0); /* scaffolding */
>> security_release_secctx(&context);
>> return ret;
>> }
>> @@ -620,12 +618,16 @@ static inline int ctnetlink_secctx_size(const struct nf_conn *ct)
>> #ifdef CONFIG_NF_CONNTRACK_SECMARK
>> int len, ret;
>> struct lsmblob blob;
>> + struct lsmcontext context;
>>
>> lsmblob_init(&blob, ct->secmark);
>> - ret = security_secid_to_secctx(&blob, NULL, &len);
>> + ret = security_secid_to_secctx(&blob, &context);
>> if (ret)
>> return 0;
>>
>> + len = context.len;
>> + security_release_secctx(&context);
>> +
>> return nla_total_size(0) /* CTA_SECCTX */
>> + nla_total_size(sizeof(char) * len); /* CTA_SECCTX_NAME */
>> #else
>> diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
>> index 79158ad0486e..fcb51ab2bb8b 100644
>> --- a/net/netfilter/nf_conntrack_standalone.c
>> +++ b/net/netfilter/nf_conntrack_standalone.c
>> @@ -173,19 +173,16 @@ static void ct_seq_stop(struct seq_file *s, void *v)
>> static void ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
>> {
>> int ret;
>> - u32 len;
>> - char *secctx;
>> struct lsmblob blob;
>> struct lsmcontext context;
>>
>> lsmblob_init(&blob, ct->secmark);
>> - ret = security_secid_to_secctx(&blob, &secctx, &len);
>> + ret = security_secid_to_secctx(&blob, &context);
>> if (ret)
>> return;
>>
>> - seq_printf(s, "secctx=%s ", secctx);
>> + seq_printf(s, "secctx=%s ", context.context);
>>
>> - lsmcontext_init(&context, secctx, len, 0); /* scaffolding */
>> security_release_secctx(&context);
>> }
>> #else
>> diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
>> index fe8403ef4e89..6da00c7add5b 100644
>> --- a/net/netfilter/nfnetlink_queue.c
>> +++ b/net/netfilter/nfnetlink_queue.c
>> @@ -310,6 +310,7 @@ static u32 nfqnl_get_sk_secctx(struct sk_buff *skb, char **secdata)
>> u32 seclen = 0;
>> #if IS_ENABLED(CONFIG_NETWORK_SECMARK)
>> struct lsmblob blob;
>> + struct lsmcontext context;
>>
>> if (!skb || !sk_fullsock(skb->sk))
>> return 0;
>> @@ -318,10 +319,12 @@ static u32 nfqnl_get_sk_secctx(struct sk_buff *skb, char **secdata)
>>
>> if (skb->secmark) {
>> lsmblob_init(&blob, skb->secmark);
>> - security_secid_to_secctx(&blob, secdata, &seclen);
>> + security_secid_to_secctx(&blob, &context);
>> + *secdata = context.context;
>> }
>>
>> read_unlock_bh(&skb->sk->sk_callback_lock);
>> + seclen = context.len;
>> #endif
>> return seclen;
>> }
>> diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
>> index 15b1945853be..4716e0011ba5 100644
>> --- a/net/netlabel/netlabel_unlabeled.c
>> +++ b/net/netlabel/netlabel_unlabeled.c
>> @@ -388,8 +388,6 @@ int netlbl_unlhsh_add(struct net *net,
>> struct netlbl_unlhsh_iface *iface;
>> struct audit_buffer *audit_buf = NULL;
>> struct lsmcontext context;
>> - char *secctx = NULL;
>> - u32 secctx_len;
>> struct lsmblob blob;
>>
>> if (addr_len != sizeof(struct in_addr) &&
>> @@ -454,12 +452,9 @@ int netlbl_unlhsh_add(struct net *net,
>> rcu_read_unlock();
>> if (audit_buf != NULL) {
>> lsmblob_init(&blob, secid);
>> - if (security_secid_to_secctx(&blob,
>> - &secctx,
>> - &secctx_len) == 0) {
>> - audit_log_format(audit_buf, " sec_obj=%s", secctx);
>> - /* scaffolding */
>> - lsmcontext_init(&context, secctx, secctx_len, 0);
>> + if (security_secid_to_secctx(&blob, &context) == 0) {
>> + audit_log_format(audit_buf, " sec_obj=%s",
>> + context.context);
>> security_release_secctx(&context);
>> }
>> audit_log_format(audit_buf, " res=%u", ret_val == 0 ? 1 : 0);
>> @@ -492,8 +487,6 @@ static int netlbl_unlhsh_remove_addr4(struct net *net,
>> struct audit_buffer *audit_buf;
>> struct net_device *dev;
>> struct lsmcontext context;
>> - char *secctx;
>> - u32 secctx_len;
>> struct lsmblob blob;
>>
>> spin_lock(&netlbl_unlhsh_lock);
>> @@ -517,11 +510,9 @@ static int netlbl_unlhsh_remove_addr4(struct net *net,
>> if (entry != NULL)
>> lsmblob_init(&blob, entry->secid);
>> if (entry != NULL &&
>> - security_secid_to_secctx(&blob,
>> - &secctx, &secctx_len) == 0) {
>> - audit_log_format(audit_buf, " sec_obj=%s", secctx);
>> - /* scaffolding */
>> - lsmcontext_init(&context, secctx, secctx_len, 0);
>> + security_secid_to_secctx(&blob, &context) == 0) {
>> + audit_log_format(audit_buf, " sec_obj=%s",
>> + context.context);
>> security_release_secctx(&context);
>> }
>> audit_log_format(audit_buf, " res=%u", entry != NULL ? 1 : 0);
>> @@ -560,8 +551,6 @@ static int netlbl_unlhsh_remove_addr6(struct net *net,
>> struct audit_buffer *audit_buf;
>> struct net_device *dev;
>> struct lsmcontext context;
>> - char *secctx;
>> - u32 secctx_len;
>> struct lsmblob blob;
>>
>> spin_lock(&netlbl_unlhsh_lock);
>> @@ -584,10 +573,9 @@ static int netlbl_unlhsh_remove_addr6(struct net *net,
>> if (entry != NULL)
>> lsmblob_init(&blob, entry->secid);
>> if (entry != NULL &&
>> - security_secid_to_secctx(&blob,
>> - &secctx, &secctx_len) == 0) {
>> - audit_log_format(audit_buf, " sec_obj=%s", secctx);
>> - lsmcontext_init(&context, secctx, secctx_len, 0);
>> + security_secid_to_secctx(&blob, &context) == 0) {
>> + audit_log_format(audit_buf, " sec_obj=%s",
>> + context.context);
>> security_release_secctx(&context);
>> }
>> audit_log_format(audit_buf, " res=%u", entry != NULL ? 1 : 0);
>> @@ -1105,8 +1093,6 @@ static int netlbl_unlabel_staticlist_gen(u32 cmd,
>> struct lsmcontext context;
>> void *data;
>> u32 secid;
>> - char *secctx;
>> - u32 secctx_len;
>> struct lsmblob blob;
>>
>> data = genlmsg_put(cb_arg->skb, NETLINK_CB(cb_arg->nl_cb->skb).portid,
>> @@ -1163,15 +1149,13 @@ static int netlbl_unlabel_staticlist_gen(u32 cmd,
>> }
>>
>> lsmblob_init(&blob, secid);
>> - ret_val = security_secid_to_secctx(&blob, &secctx, &secctx_len);
>> + ret_val = security_secid_to_secctx(&blob, &context);
>> if (ret_val != 0)
>> goto list_cb_failure;
>> ret_val = nla_put(cb_arg->skb,
>> NLBL_UNLABEL_A_SECCTX,
>> - secctx_len,
>> - secctx);
>> - /* scaffolding */
>> - lsmcontext_init(&context, secctx, secctx_len, 0);
>> + context.len,
>> + context.context);
>> security_release_secctx(&context);
>> if (ret_val != 0)
>> goto list_cb_failure;
>> diff --git a/net/netlabel/netlabel_user.c b/net/netlabel/netlabel_user.c
>> index 94aea4985b74..2d1307f65250 100644
>> --- a/net/netlabel/netlabel_user.c
>> +++ b/net/netlabel/netlabel_user.c
>> @@ -99,8 +99,6 @@ struct audit_buffer *netlbl_audit_start_common(int type,
>> {
>> struct audit_buffer *audit_buf;
>> struct lsmcontext context;
>> - char *secctx;
>> - u32 secctx_len;
>> struct lsmblob blob;
>>
>> if (audit_enabled == AUDIT_OFF)
>> @@ -116,9 +114,8 @@ struct audit_buffer *netlbl_audit_start_common(int type,
>>
>> lsmblob_init(&blob, audit_info->secid);
>> if (audit_info->secid != 0 &&
>> - security_secid_to_secctx(&blob, &secctx, &secctx_len) == 0) {
>> - audit_log_format(audit_buf, " subj=%s", secctx);
>> - lsmcontext_init(&context, secctx, secctx_len, 0);/*scaffolding*/
>> + security_secid_to_secctx(&blob, &context) == 0) {
>> + audit_log_format(audit_buf, " subj=%s", context.context);
>> security_release_secctx(&context);
>> }
>>
>> diff --git a/security/security.c b/security/security.c
>> index 3563b1e2f8f9..97b468f6e6a9 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -2081,7 +2081,7 @@ int security_ismaclabel(const char *name)
>> }
>> EXPORT_SYMBOL(security_ismaclabel);
>>
>> -int security_secid_to_secctx(struct lsmblob *blob, char **secdata, u32 *seclen)
>> +int security_secid_to_secctx(struct lsmblob *blob, struct lsmcontext *cp)
>> {
>> struct security_hook_list *hp;
>> int *display = current->security;
>> @@ -2089,10 +2089,13 @@ int security_secid_to_secctx(struct lsmblob *blob, char **secdata, u32 *seclen)
>> hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list) {
>> if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
>> continue;
>> - if (*display == LSMBLOB_INVALID || *display == hp->lsmid->slot)
>> + if (*display == LSMBLOB_INVALID ||
>> + *display == hp->lsmid->slot) {
>> + cp->slot = hp->lsmid->slot;
>> return hp->hook.secid_to_secctx(
>> blob->secid[hp->lsmid->slot],
>> - secdata, seclen);
>> + &cp->context, &cp->len);
>> + }
>> }
>> return 0;
>> }
>>
>
>
^ permalink raw reply
* Re: [PATCH v4 19/23] LSM: Use lsmcontext in security_inode_getsecctx
From: John Johansen @ 2019-06-27 21:36 UTC (permalink / raw)
To: Casey Schaufler, casey.schaufler, jmorris, linux-security-module,
selinux
Cc: keescook, penguin-kernel, paul, sds
In-Reply-To: <20190626192234.11725-20-casey@schaufler-ca.com>
On 6/26/19 12:22 PM, Casey Schaufler wrote:
> Change the security_inode_getsecctx() interface to fill
> a lsmcontext structure instead of data and length pointers.
> This provides the information about which LSM created the
> context so that security_release_secctx() can use the
> correct hook. A lsmcontext is used within kernfs to store
> the security information as well.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Reviewed-by: John Johansen <john.johansen@canonical.com>
> ---
> fs/kernfs/dir.c | 8 ++------
> fs/kernfs/inode.c | 34 ++++++++++++----------------------
> fs/kernfs/kernfs-internal.h | 3 +--
> fs/nfsd/nfs4xdr.c | 23 +++++++++--------------
> include/linux/security.h | 5 +++--
> security/security.c | 14 ++++++++++++--
> 6 files changed, 39 insertions(+), 48 deletions(-)
>
> diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
> index 92afad387237..1d000289d8b7 100644
> --- a/fs/kernfs/dir.c
> +++ b/fs/kernfs/dir.c
> @@ -532,12 +532,8 @@ void kernfs_put(struct kernfs_node *kn)
> kfree_const(kn->name);
>
> if (kn->iattr) {
> - struct lsmcontext scaff; /* scaffolding */
> - if (kn->iattr->ia_secdata) {
> - lsmcontext_init(&scaff, kn->iattr->ia_secdata,
> - kn->iattr->ia_secdata_len, 0);
> - security_release_secctx(&scaff);
> - }
> + if (kn->iattr->ia_context.context)
> + security_release_secctx(&kn->iattr->ia_context);
> simple_xattrs_free(&kn->iattr->xattrs);
> kmem_cache_free(kernfs_iattrs_cache, kn->iattr);
> }
> diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c
> index 02cde9dac5ee..ffbf7863306d 100644
> --- a/fs/kernfs/inode.c
> +++ b/fs/kernfs/inode.c
> @@ -135,21 +135,14 @@ int kernfs_iop_setattr(struct dentry *dentry, struct iattr *iattr)
> return error;
> }
>
> -static int kernfs_node_setsecdata(struct kernfs_iattrs *attrs, void **secdata,
> - u32 *secdata_len)
> +static void kernfs_node_setsecdata(struct kernfs_iattrs *attrs,
> + struct lsmcontext *cp)
> {
> - void *old_secdata;
> - size_t old_secdata_len;
> + struct lsmcontext old_context;
>
> - old_secdata = attrs->ia_secdata;
> - old_secdata_len = attrs->ia_secdata_len;
> -
> - attrs->ia_secdata = *secdata;
> - attrs->ia_secdata_len = *secdata_len;
> -
> - *secdata = old_secdata;
> - *secdata_len = old_secdata_len;
> - return 0;
> + old_context = attrs->ia_context;
> + attrs->ia_context = *cp;
> + *cp = old_context;
> }
>
> ssize_t kernfs_iop_listxattr(struct dentry *dentry, char *buf, size_t size)
> @@ -192,8 +185,8 @@ static void kernfs_refresh_inode(struct kernfs_node *kn, struct inode *inode)
> * persistent copy in kernfs_node.
> */
> set_inode_attr(inode, &attrs->ia_iattr);
> - security_inode_notifysecctx(inode, attrs->ia_secdata,
> - attrs->ia_secdata_len);
> + security_inode_notifysecctx(inode, attrs->ia_context.context,
> + attrs->ia_context.len);
> }
>
> if (kernfs_type(kn) == KERNFS_DIR)
> @@ -350,8 +343,6 @@ static int kernfs_security_xattr_set(const struct xattr_handler *handler,
> struct kernfs_node *kn = inode->i_private;
> struct kernfs_iattrs *attrs;
> struct lsmcontext context;
> - void *secdata;
> - u32 secdata_len = 0;
> int error;
>
> attrs = kernfs_iattrs(kn);
> @@ -361,18 +352,17 @@ static int kernfs_security_xattr_set(const struct xattr_handler *handler,
> error = security_inode_setsecurity(inode, suffix, value, size, flags);
> if (error)
> return error;
> - error = security_inode_getsecctx(inode, &secdata, &secdata_len);
> + error = security_inode_getsecctx(inode, &context);
> if (error)
> return error;
>
> mutex_lock(&kernfs_mutex);
> - error = kernfs_node_setsecdata(attrs, &secdata, &secdata_len);
> + kernfs_node_setsecdata(attrs, &context);
> mutex_unlock(&kernfs_mutex);
>
> - if (secdata) {
> - lsmcontext_init(&context, secdata, secdata_len, 0);
> + if (context.context)
> security_release_secctx(&context);
> - }
> +
> return error;
> }
>
> diff --git a/fs/kernfs/kernfs-internal.h b/fs/kernfs/kernfs-internal.h
> index 0b7d197a904c..844a028d282f 100644
> --- a/fs/kernfs/kernfs-internal.h
> +++ b/fs/kernfs/kernfs-internal.h
> @@ -21,8 +21,7 @@
>
> struct kernfs_iattrs {
> struct iattr ia_iattr;
> - void *ia_secdata;
> - u32 ia_secdata_len;
> + struct lsmcontext ia_context;
>
> struct simple_xattrs xattrs;
> };
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index bb3db033e144..1209083565dd 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -2304,11 +2304,11 @@ nfsd4_encode_layout_types(struct xdr_stream *xdr, u32 layout_types)
> #ifdef CONFIG_NFSD_V4_SECURITY_LABEL
> static inline __be32
> nfsd4_encode_security_label(struct xdr_stream *xdr, struct svc_rqst *rqstp,
> - void *context, int len)
> + struct lsmcontext *context)
> {
> __be32 *p;
>
> - p = xdr_reserve_space(xdr, len + 4 + 4 + 4);
> + p = xdr_reserve_space(xdr, context->len + 4 + 4 + 4);
> if (!p)
> return nfserr_resource;
>
> @@ -2318,13 +2318,13 @@ nfsd4_encode_security_label(struct xdr_stream *xdr, struct svc_rqst *rqstp,
> */
> *p++ = cpu_to_be32(0); /* lfs */
> *p++ = cpu_to_be32(0); /* pi */
> - p = xdr_encode_opaque(p, context, len);
> + p = xdr_encode_opaque(p, context->context, context->len);
> return 0;
> }
> #else
> static inline __be32
> nfsd4_encode_security_label(struct xdr_stream *xdr, struct svc_rqst *rqstp,
> - void *context, int len)
> + struct lsmcontext *context)
> { return 0; }
> #endif
>
> @@ -2420,9 +2420,7 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
> __be32 status;
> int err;
> struct nfs4_acl *acl = NULL;
> - struct lsmcontext scaff; /* scaffolding */
> - void *context = NULL;
> - int contextlen;
> + struct lsmcontext context;
> bool contextsupport = false;
> struct nfsd4_compoundres *resp = rqstp->rq_resp;
> u32 minorversion = resp->cstate.minorversion;
> @@ -2479,7 +2477,7 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
> bmval0 & FATTR4_WORD0_SUPPORTED_ATTRS) {
> if (exp->ex_flags & NFSEXP_SECURITY_LABEL)
> err = security_inode_getsecctx(d_inode(dentry),
> - &context, &contextlen);
> + &context);
> else
> err = -EOPNOTSUPP;
> contextsupport = (err == 0);
> @@ -2908,8 +2906,7 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
> }
>
> if (bmval2 & FATTR4_WORD2_SECURITY_LABEL) {
> - status = nfsd4_encode_security_label(xdr, rqstp, context,
> - contextlen);
> + status = nfsd4_encode_security_label(xdr, rqstp, &context);
> if (status)
> goto out;
> }
> @@ -2920,10 +2917,8 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
>
> out:
> #ifdef CONFIG_NFSD_V4_SECURITY_LABEL
> - if (context) {
> - lsmcontext_init(&scaff, context, contextlen, 0); /*scaffolding*/
> - security_release_secctx(&scaff);
> - }
> + if (context.context)
> + security_release_secctx(&context);
> #endif /* CONFIG_NFSD_V4_SECURITY_LABEL */
> kfree(acl);
> if (tempfh) {
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 2674eb70c2d7..c16aea55be97 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -489,7 +489,7 @@ void security_release_secctx(struct lsmcontext *cp);
> void security_inode_invalidate_secctx(struct inode *inode);
> int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen);
> int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen);
> -int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen);
> +int security_inode_getsecctx(struct inode *inode, struct lsmcontext *cp);
> #else /* CONFIG_SECURITY */
>
> static inline int call_lsm_notifier(enum lsm_event event, void *data)
> @@ -1290,7 +1290,8 @@ static inline int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32
> {
> return -EOPNOTSUPP;
> }
> -static inline int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen)
> +static inline int security_inode_getsecctx(struct inode *inode,
> + struct lsmcontext *cp)
> {
> return -EOPNOTSUPP;
> }
> diff --git a/security/security.c b/security/security.c
> index 61cdc6bcd32e..45b9f905f5c0 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -2164,9 +2164,19 @@ int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen)
> }
> EXPORT_SYMBOL(security_inode_setsecctx);
>
> -int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen)
> +int security_inode_getsecctx(struct inode *inode, struct lsmcontext *cp)
> {
> - return call_int_hook(inode_getsecctx, -EOPNOTSUPP, inode, ctx, ctxlen);
> + int *display = current->security;
> + struct security_hook_list *hp;
> +
> + hlist_for_each_entry(hp, &security_hook_heads.inode_getsecctx, list)
> + if (*display == LSMBLOB_INVALID ||
> + *display == hp->lsmid->slot) {
> + cp->slot = hp->lsmid->slot;
> + return hp->hook.inode_getsecctx(inode,
> + (void **)&cp->context, &cp->len);
> + }
> + return -EOPNOTSUPP;
> }
> EXPORT_SYMBOL(security_inode_getsecctx);
>
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox