Re: [PATCH] selinux: Add __GFP_NOWARN to allocation at str_read()
On 09/13/2018 01:11 PM, Michal Hocko wrote: > On Thu 13-09-18 09:12:04, peter enderborg wrote: >> On 09/13/2018 08:26 AM, Tetsuo Handa wrote: >>> On 2018/09/13 12:02, Paul Moore wrote: >>>> On Fri, Sep 7, 2018 at 12:43 PM Tetsuo Handa >>>> wrote: >>>>> syzbot is hitting warning at str_read() [1] because len parameter can >>>>> become larger than KMALLOC_MAX_SIZE. We don't need to emit warning for >>>>> this case. >>>>> >>>>> [1] >>>>> https://syzkaller.appspot.com/bug?id=7f2f5aad79ea8663c296a2eedb81978401a908f0 >>>>> >>>>> Signed-off-by: Tetsuo Handa >>>>> Reported-by: syzbot >>>>> >>>>> --- >>>>> security/selinux/ss/policydb.c | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/security/selinux/ss/policydb.c >>>>> b/security/selinux/ss/policydb.c >>>>> index e9394e7..f4eadd3 100644 >>>>> --- a/security/selinux/ss/policydb.c >>>>> +++ b/security/selinux/ss/policydb.c >>>>> @@ -1101,7 +1101,7 @@ static int str_read(char **strp, gfp_t flags, void >>>>> *fp, u32 len) >>>>> if ((len == 0) || (len == (u32)-1)) >>>>> return -EINVAL; >>>>> >>>>> - str = kmalloc(len + 1, flags); >>>>> + str = kmalloc(len + 1, flags | __GFP_NOWARN); >>>>> if (!str) >>>>> return -ENOMEM; >>>> Thanks for the patch. >>>> >>>> My eyes are starting to glaze over a bit chasing down all of the >>>> different kmalloc() code paths trying to ensure that this always does >>>> the right thing based on size of the allocation and the different slab >>>> allocators ... are we sure that this will always return NULL when (len >>>> + 1) is greater than KMALLOC_MAX_SIZE for the different slab allocator >>>> configurations? >>>> >>> Yes, for (len + 1) cannot become 0 (which causes kmalloc() to return >>> ZERO_SIZE_PTR) due to (len == (u32)-1) check above. >>> >>> The only concern would be whether you want allocation failure messages. >>> I assumed you don't need it because we are returning -ENOMEM to the caller. >>> >> Would it not be better with >> >> char *str; >> >> if ((len == 0) || (len == (u32)-1) || (len >= KMALLOC_MAX_SIZE)) >> return -EINVAL; >> >> str = kmalloc(len + 1, flags); >> if (!str) >> return -ENOMEM; > I strongly suspect that you want kvmalloc rather than kmalloc here. The > larger the request the more likely is the allocation to fail. > > I am not familiar with the code but I assume this is a root only > interface so we don't have to worry about nasty users scenario. > I don't think we get any big data there at all. Usually less than 32 bytes. However this data can be in fast path so a vmalloc is not an option. And some of the calls are GFP_ATOMC. ___ Selinux mailing list Selinux@tycho.nsa.gov To unsubscribe, send email to selinux-le...@tycho.nsa.gov. To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: [PATCH] selinux: Add __GFP_NOWARN to allocation at str_read()
On 09/13/2018 08:26 AM, Tetsuo Handa wrote: > On 2018/09/13 12:02, Paul Moore wrote: >> On Fri, Sep 7, 2018 at 12:43 PM Tetsuo Handa >> wrote: >>> syzbot is hitting warning at str_read() [1] because len parameter can >>> become larger than KMALLOC_MAX_SIZE. We don't need to emit warning for >>> this case. >>> >>> [1] >>> https://syzkaller.appspot.com/bug?id=7f2f5aad79ea8663c296a2eedb81978401a908f0 >>> >>> Signed-off-by: Tetsuo Handa >>> Reported-by: syzbot >>> --- >>> security/selinux/ss/policydb.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c >>> index e9394e7..f4eadd3 100644 >>> --- a/security/selinux/ss/policydb.c >>> +++ b/security/selinux/ss/policydb.c >>> @@ -1101,7 +1101,7 @@ static int str_read(char **strp, gfp_t flags, void >>> *fp, u32 len) >>> if ((len == 0) || (len == (u32)-1)) >>> return -EINVAL; >>> >>> - str = kmalloc(len + 1, flags); >>> + str = kmalloc(len + 1, flags | __GFP_NOWARN); >>> if (!str) >>> return -ENOMEM; >> Thanks for the patch. >> >> My eyes are starting to glaze over a bit chasing down all of the >> different kmalloc() code paths trying to ensure that this always does >> the right thing based on size of the allocation and the different slab >> allocators ... are we sure that this will always return NULL when (len >> + 1) is greater than KMALLOC_MAX_SIZE for the different slab allocator >> configurations? >> > Yes, for (len + 1) cannot become 0 (which causes kmalloc() to return > ZERO_SIZE_PTR) due to (len == (u32)-1) check above. > > The only concern would be whether you want allocation failure messages. > I assumed you don't need it because we are returning -ENOMEM to the caller. > Would it not be better with char *str; if ((len == 0) || (len == (u32)-1) || (len >= KMALLOC_MAX_SIZE)) return -EINVAL; str = kmalloc(len + 1, flags); if (!str) return -ENOMEM; ___ Selinux mailing list Selinux@tycho.nsa.gov To unsubscribe, send email to selinux-le...@tycho.nsa.gov. To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: [PATCH 01/13] selinux: Cleanup printk logging in conditional
On 06/12/2018 04:38 PM, Joe Perches wrote:
> On Tue, 2018-06-12 at 10:09 +0200, Peter Enderborg wrote:
>> Replace printk with pr_* to avoid checkpatch warnings.
> I believe it would be nicer to remove the
> "SELinux: " prefix embbeded in each format
> and use a specific
>
> #define pr_fmt(fmt) "SELinux: " fmt
>
> to automatically prefix these formats.
I cant argument about that, however some of the warnings and debug prints in
this set does not have this
so it will then change the actual output. (And I also think that they should
have a the prefix, but I don't
know why they don't) So I am not sure if it appropriate for a cleanup patch, it
supposed to have no functional change.
>> diff --git a/security/selinux/ss/conditional.c
>> b/security/selinux/ss/conditional.c
> []
>> @@ -96,7 +96,7 @@ int evaluate_cond_node(struct policydb *p, struct
>> cond_node *node)
>> if (new_state != node->cur_state) {
>> node->cur_state = new_state;
>> if (new_state == -1)
>> -printk(KERN_ERR "SELinux: expression result was
>> undefined - disabling all rules.\n");
>> +pr_err("SELinux: expression result was undefined -
>> disabling all rules.\n");
>> /* turn the rules on or off */
>> for (cur = node->true_list; cur; cur = cur->next) {
>> if (new_state <= 0)
> So, for instance, this patch could become:
> (etc and so forth for each patch in this series)
>
> ---
> security/selinux/ss/conditional.c | 18 ++
> 1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/security/selinux/ss/conditional.c
> b/security/selinux/ss/conditional.c
> index c91543a617ac..e96820d92b61 100644
> --- a/security/selinux/ss/conditional.c
> +++ b/security/selinux/ss/conditional.c
> @@ -7,6 +7,8 @@
> * the Free Software Foundation, version 2.
> */
>
> +#define pr_fmt(fmt) "SELinux: " fmt
> +
> #include
> #include
> #include
> @@ -96,7 +98,7 @@ int evaluate_cond_node(struct policydb *p, struct cond_node
> *node)
> if (new_state != node->cur_state) {
> node->cur_state = new_state;
> if (new_state == -1)
> - printk(KERN_ERR "SELinux: expression result was
> undefined - disabling all rules.\n");
> + pr_err("expression result was undefined - disabling all
> rules\n");
> /* turn the rules on or off */
> for (cur = node->true_list; cur; cur = cur->next) {
> if (new_state <= 0)
> @@ -287,7 +289,7 @@ static int cond_insertf(struct avtab *a, struct avtab_key
> *k, struct avtab_datum
>*/
> if (k->specified & AVTAB_TYPE) {
> if (avtab_search(>te_avtab, k)) {
> - printk(KERN_ERR "SELinux: type rule already exists
> outside of a conditional.\n");
> + pr_err("type rule already exists outside of a
> conditional\n");
> goto err;
> }
> /*
> @@ -302,7 +304,7 @@ static int cond_insertf(struct avtab *a, struct avtab_key
> *k, struct avtab_datum
> node_ptr = avtab_search_node(>te_cond_avtab, k);
> if (node_ptr) {
> if (avtab_search_node_next(node_ptr,
> k->specified)) {
> - printk(KERN_ERR "SELinux: too many
> conflicting type rules.\n");
> + pr_err("too many conflicting type
> rules\n");
> goto err;
> }
> found = 0;
> @@ -313,13 +315,13 @@ static int cond_insertf(struct avtab *a, struct
> avtab_key *k, struct avtab_datum
> }
> }
> if (!found) {
> - printk(KERN_ERR "SELinux: conflicting
> type rules.\n");
> + pr_err("conflicting type rules\n");
> goto err;
> }
> }
> } else {
> if (avtab_search(>te_cond_avtab, k)) {
> - printk(KERN_ERR "SELinux: conflicting type
> rules when adding type rule for true.\n");
> + pr_err("conflicting type rules when adding t
[PATCH 11/13] selinux: Cleanup printk logging in netif
Replace printk with pr_* to avoid checkpatch warnings.
Signed-off-by: Peter Enderborg
---
security/selinux/netif.c | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/security/selinux/netif.c b/security/selinux/netif.c
index ac65f7417413..8c738c189942 100644
--- a/security/selinux/netif.c
+++ b/security/selinux/netif.c
@@ -145,9 +145,8 @@ static int sel_netif_sid_slow(struct net *ns, int ifindex,
u32 *sid)
dev = dev_get_by_index(ns, ifindex);
if (unlikely(dev == NULL)) {
- printk(KERN_WARNING
- "SELinux: failure in sel_netif_sid_slow(),"
- " invalid network interface (%d)\n", ifindex);
+ pr_warn("SELinux: failure in %s(), invalid network interface
(%d)\n",
+ __func__, ifindex);
return -ENOENT;
}
@@ -177,10 +176,8 @@ static int sel_netif_sid_slow(struct net *ns, int ifindex,
u32 *sid)
spin_unlock_bh(_netif_lock);
dev_put(dev);
if (unlikely(ret)) {
- printk(KERN_WARNING
- "SELinux: failure in sel_netif_sid_slow(),"
- " unable to determine network interface label (%d)\n",
- ifindex);
+ pr_warn("SELinux: failure in %s(), unable to determine network
interface label (%d)\n",
+ __func__, ifindex);
kfree(new);
}
return ret;
--
2.15.1
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
[PATCH 00/13 selinux-next] selinux: Cleanup printk logging
This patch replaces printk with pr_* for the selinux files. I get a lot of checkpatch warnings when doing my other work, lets get rid of the warnings. For the policydb.c there also a removal of KERN_CONT with two longer prints. I have NOT cleaned up splitting lines with long prints. I think the current conclusion is that it is better to have long lines that it to have splitting print lines. There is one patch per file for this files: conditional.c ebitmap.c policydb.c avtab.c hooks.c avtab.c services.c selinuxfs.c netlink.c sidtab.c netport.c netif.c avc.c netnode.c ___ Selinux mailing list Selinux@tycho.nsa.gov To unsubscribe, send email to selinux-le...@tycho.nsa.gov. To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
[PATCH 03/13] selinux: Cleanup printk logging in policydb
Replace printk with pr_* to avoid checkpatch warnings and
replace KERN_CONT with 2 longer prints.
Signed-off-by: Peter Enderborg
---
security/selinux/ss/policydb.c | 91 +-
1 file changed, 46 insertions(+), 45 deletions(-)
diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index 6e8c8056d7ad..4e82c5fcd1a1 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -504,7 +504,7 @@ static void hash_eval(struct hashtab *h, const char
*hash_name)
struct hashtab_info info;
hashtab_stat(h, );
- printk(KERN_DEBUG "SELinux: %s: %d entries and %d/%d buckets used, "
+ pr_debug("SELinux: %s: %d entries and %d/%d buckets used, "
"longest chain length %d\n", hash_name, h->nel,
info.slots_used, h->size, info.max_chain_len);
}
@@ -533,15 +533,17 @@ static int policydb_index(struct policydb *p)
{
int i, rc;
- printk(KERN_DEBUG "SELinux: %d users, %d roles, %d types, %d bools",
- p->p_users.nprim, p->p_roles.nprim, p->p_types.nprim,
p->p_bools.nprim);
if (p->mls_enabled)
- printk(KERN_CONT ", %d sens, %d cats", p->p_levels.nprim,
- p->p_cats.nprim);
- printk(KERN_CONT "\n");
+ pr_debug("SELinux: %d users, %d roles, %d types, %d bools, %d
sens, %d cats",
+p->p_users.nprim, p->p_roles.nprim, p->p_types.nprim,
+p->p_bools.nprim, p->p_levels.nprim, p->p_cats.nprim);
+ else
+ pr_debug("SELinux: %d users, %d roles, %d types, %d bools",
+p->p_users.nprim, p->p_roles.nprim, p->p_types.nprim,
+p->p_bools.nprim);
- printk(KERN_DEBUG "SELinux: %d classes, %d rules\n",
- p->p_classes.nprim, p->te_avtab.nel);
+ pr_debug("SELinux: %d classes, %d rules\n",
+p->p_classes.nprim, p->te_avtab.nel);
#ifdef DEBUG_HASHES
avtab_hash_eval(>te_avtab, "rules");
@@ -897,7 +899,7 @@ int policydb_load_isids(struct policydb *p, struct sidtab
*s)
rc = sidtab_init(s);
if (rc) {
- printk(KERN_ERR "SELinux: out of memory on SID table init\n");
+ pr_err("SELinux: out of memory on SID table init\n");
goto out;
}
@@ -905,14 +907,14 @@ int policydb_load_isids(struct policydb *p, struct sidtab
*s)
for (c = head; c; c = c->next) {
rc = -EINVAL;
if (!c->context[0].user) {
- printk(KERN_ERR "SELinux: SID %s was never defined.\n",
+ pr_err("SELinux: SID %s was never defined.\n",
c->u.name);
goto out;
}
rc = sidtab_insert(s, c->sid[0], >context[0]);
if (rc) {
- printk(KERN_ERR "SELinux: unable to load initial SID
%s.\n",
+ pr_err("SELinux: unable to load initial SID %s.\n",
c->u.name);
goto out;
}
@@ -1005,13 +1007,13 @@ static int mls_read_range_helper(struct mls_range *r,
void *fp)
rc = -EINVAL;
items = le32_to_cpu(buf[0]);
if (items > ARRAY_SIZE(buf)) {
- printk(KERN_ERR "SELinux: mls: range overflow\n");
+ pr_err("SELinux: mls: range overflow\n");
goto out;
}
rc = next_entry(buf, fp, sizeof(u32) * items);
if (rc) {
- printk(KERN_ERR "SELinux: mls: truncated range\n");
+ pr_err("SELinux: mls: truncated range\n");
goto out;
}
@@ -1023,19 +1025,19 @@ static int mls_read_range_helper(struct mls_range *r,
void *fp)
rc = ebitmap_read(>level[0].cat, fp);
if (rc) {
- printk(KERN_ERR "SELinux: mls: error reading low
categories\n");
+ pr_err("SELinux: mls: error reading low categories\n");
goto out;
}
if (items > 1) {
rc = ebitmap_read(>level[1].cat, fp);
if (rc) {
- printk(KERN_ERR "SELinux: mls: error reading high
categories\n");
+ pr_err("SELinux: mls: error reading high
categories\n");
goto bad_high;
}
} else {
rc = ebitmap_cpy(>level[1].cat, >level[0].cat);
if (rc) {
- printk(KERN_ERR "SELinux:
[PATCH 01/13] selinux: Cleanup printk logging in conditional
Replace printk with pr_* to avoid checkpatch warnings.
Signed-off-by: Peter Enderborg
---
security/selinux/ss/conditional.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/security/selinux/ss/conditional.c
b/security/selinux/ss/conditional.c
index c91543a617ac..f49e522e932d 100644
--- a/security/selinux/ss/conditional.c
+++ b/security/selinux/ss/conditional.c
@@ -96,7 +96,7 @@ int evaluate_cond_node(struct policydb *p, struct cond_node
*node)
if (new_state != node->cur_state) {
node->cur_state = new_state;
if (new_state == -1)
- printk(KERN_ERR "SELinux: expression result was
undefined - disabling all rules.\n");
+ pr_err("SELinux: expression result was undefined -
disabling all rules.\n");
/* turn the rules on or off */
for (cur = node->true_list; cur; cur = cur->next) {
if (new_state <= 0)
@@ -287,7 +287,7 @@ static int cond_insertf(struct avtab *a, struct avtab_key
*k, struct avtab_datum
*/
if (k->specified & AVTAB_TYPE) {
if (avtab_search(>te_avtab, k)) {
- printk(KERN_ERR "SELinux: type rule already exists
outside of a conditional.\n");
+ pr_err("SELinux: type rule already exists outside of a
conditional.\n");
goto err;
}
/*
@@ -302,7 +302,7 @@ static int cond_insertf(struct avtab *a, struct avtab_key
*k, struct avtab_datum
node_ptr = avtab_search_node(>te_cond_avtab, k);
if (node_ptr) {
if (avtab_search_node_next(node_ptr,
k->specified)) {
- printk(KERN_ERR "SELinux: too many
conflicting type rules.\n");
+ pr_err("SELinux: too many conflicting
type rules.\n");
goto err;
}
found = 0;
@@ -313,13 +313,13 @@ static int cond_insertf(struct avtab *a, struct avtab_key
*k, struct avtab_datum
}
}
if (!found) {
- printk(KERN_ERR "SELinux: conflicting
type rules.\n");
+ pr_err("SELinux: conflicting type
rules.\n");
goto err;
}
}
} else {
if (avtab_search(>te_cond_avtab, k)) {
- printk(KERN_ERR "SELinux: conflicting type
rules when adding type rule for true.\n");
+ pr_err("SELinux: conflicting type rules when
adding type rule for true.\n");
goto err;
}
}
@@ -327,7 +327,7 @@ static int cond_insertf(struct avtab *a, struct avtab_key
*k, struct avtab_datum
node_ptr = avtab_insert_nonunique(>te_cond_avtab, k, d);
if (!node_ptr) {
- printk(KERN_ERR "SELinux: could not insert rule.\n");
+ pr_err("SELinux: could not insert rule.\n");
rc = -ENOMEM;
goto err;
}
@@ -387,12 +387,12 @@ static int cond_read_av_list(struct policydb *p, void
*fp, struct cond_av_list *
static int expr_isvalid(struct policydb *p, struct cond_expr *expr)
{
if (expr->expr_type <= 0 || expr->expr_type > COND_LAST) {
- printk(KERN_ERR "SELinux: conditional expressions uses unknown
operator.\n");
+ pr_err("SELinux: conditional expressions uses unknown
operator.\n");
return 0;
}
if (expr->bool > p->p_bools.nprim) {
- printk(KERN_ERR "SELinux: conditional expressions uses unknown
bool.\n");
+ pr_err("SELinux: conditional expressions uses unknown bool.\n");
return 0;
}
return 1;
--
2.15.1
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
[PATCH 02/13] selinux: Cleanup printk logging in ebitmap
Replace printk with pr_* to avoid checkpatch warnings.
Signed-off-by: Peter Enderborg
---
security/selinux/ss/ebitmap.c | 15 +++
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/security/selinux/ss/ebitmap.c b/security/selinux/ss/ebitmap.c
index 5ae8c61b75bf..8f624f80055b 100644
--- a/security/selinux/ss/ebitmap.c
+++ b/security/selinux/ss/ebitmap.c
@@ -362,7 +362,7 @@ int ebitmap_read(struct ebitmap *e, void *fp)
count = le32_to_cpu(buf[2]);
if (mapunit != BITS_PER_U64) {
- printk(KERN_ERR "SELinux: ebitmap: map size %u does not "
+ pr_err("SELinux: ebitmap: map size %u does not "
"match my size %zd (high bit was %d)\n",
mapunit, BITS_PER_U64, e->highbit);
goto bad;
@@ -383,19 +383,19 @@ int ebitmap_read(struct ebitmap *e, void *fp)
for (i = 0; i < count; i++) {
rc = next_entry(, fp, sizeof(u32));
if (rc < 0) {
- printk(KERN_ERR "SELinux: ebitmap: truncated map\n");
+ pr_err("SELinux: ebitmap: truncated map\n");
goto bad;
}
startbit = le32_to_cpu(startbit);
if (startbit & (mapunit - 1)) {
- printk(KERN_ERR "SELinux: ebitmap start bit (%d) is "
+ pr_err("SELinux: ebitmap start bit (%d) is "
"not a multiple of the map unit size (%u)\n",
startbit, mapunit);
goto bad;
}
if (startbit > e->highbit - mapunit) {
- printk(KERN_ERR "SELinux: ebitmap start bit (%d) is "
+ pr_err("SELinux: ebitmap start bit (%d) is "
"beyond the end of the bitmap (%u)\n",
startbit, (e->highbit - mapunit));
goto bad;
@@ -405,8 +405,7 @@ int ebitmap_read(struct ebitmap *e, void *fp)
struct ebitmap_node *tmp;
tmp = kmem_cache_zalloc(ebitmap_node_cachep,
GFP_KERNEL);
if (!tmp) {
- printk(KERN_ERR
- "SELinux: ebitmap: out of memory\n");
+ pr_err("SELinux: ebitmap: out of memory\n");
rc = -ENOMEM;
goto bad;
}
@@ -418,7 +417,7 @@ int ebitmap_read(struct ebitmap *e, void *fp)
e->node = tmp;
n = tmp;
} else if (startbit <= n->startbit) {
- printk(KERN_ERR "SELinux: ebitmap: start bit %d"
+ pr_err("SELinux: ebitmap: start bit %d"
" comes after start bit %d\n",
startbit, n->startbit);
goto bad;
@@ -426,7 +425,7 @@ int ebitmap_read(struct ebitmap *e, void *fp)
rc = next_entry(, fp, sizeof(u64));
if (rc < 0) {
- printk(KERN_ERR "SELinux: ebitmap: truncated map\n");
+ pr_err("SELinux: ebitmap: truncated map\n");
goto bad;
}
map = le64_to_cpu(map);
--
2.15.1
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
[PATCH 08/13] selinux: Cleanup printk logging in netlink
Replace printk with pr_* to avoid checkpatch warnings.
Signed-off-by: Peter Enderborg
---
security/selinux/netlink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/selinux/netlink.c b/security/selinux/netlink.c
index 828fb6a4e941..8a8a72507437 100644
--- a/security/selinux/netlink.c
+++ b/security/selinux/netlink.c
@@ -94,7 +94,7 @@ static void selnl_notify(int msgtype, void *data)
out_kfree_skb:
kfree_skb(skb);
oom:
- printk(KERN_ERR "SELinux: OOM in %s\n", __func__);
+ pr_err("SELinux: OOM in %s\n", __func__);
goto out;
}
--
2.15.1
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
[PATCH 07/13] selinux: Cleanup printk logging in selinuxfs
Replace printk with pr_* to avoid checkpatch warnings.
Signed-off-by: Peter Enderborg
---
security/selinux/selinuxfs.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index c0cadbc5f85c..2adfade99945 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -620,7 +620,7 @@ static ssize_t sel_write_context(struct file *file, char
*buf, size_t size)
length = -ERANGE;
if (len > SIMPLE_TRANSACTION_LIMIT) {
- printk(KERN_ERR "SELinux: %s: context size (%u) exceeds "
+ pr_err("SELinux: %s: context size (%u) exceeds "
"payload max\n", __func__, len);
goto out;
}
@@ -956,7 +956,7 @@ static ssize_t sel_write_create(struct file *file, char
*buf, size_t size)
length = -ERANGE;
if (len > SIMPLE_TRANSACTION_LIMIT) {
- printk(KERN_ERR "SELinux: %s: context size (%u) exceeds "
+ pr_err("SELinux: %s: context size (%u) exceeds "
"payload max\n", __func__, len);
goto out;
}
@@ -1147,7 +1147,7 @@ static ssize_t sel_write_member(struct file *file, char
*buf, size_t size)
length = -ERANGE;
if (len > SIMPLE_TRANSACTION_LIMIT) {
- printk(KERN_ERR "SELinux: %s: context size (%u) exceeds "
+ pr_err("SELinux: %s: context size (%u) exceeds "
"payload max\n", __func__, len);
goto out;
}
@@ -1996,7 +1996,7 @@ static int sel_fill_super(struct super_block *sb, void
*data, int silent)
goto err;
return 0;
err:
- printk(KERN_ERR "SELinux: %s: failed while creating inodes\n",
+ pr_err("SELinux: %s: failed while creating inodes\n",
__func__);
selinux_fs_info_free(sb);
@@ -2046,7 +2046,7 @@ static int __init init_sel_fs(void)
selinux_null.mnt = selinuxfs_mount = kern_mount(_fs_type);
if (IS_ERR(selinuxfs_mount)) {
- printk(KERN_ERR "selinuxfs: could not mount!\n");
+ pr_err("selinuxfs: could not mount!\n");
err = PTR_ERR(selinuxfs_mount);
selinuxfs_mount = NULL;
}
--
2.15.1
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
[PATCH 06/13] selinux: Cleanup printk logging in services
Replace printk with pr_* to avoid checkpatch warnings.
Signed-off-by: Peter Enderborg
---
security/selinux/ss/services.c | 71 +-
1 file changed, 35 insertions(+), 36 deletions(-)
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 8057e19dc15f..9ad9b6c2f0a7 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -136,8 +136,7 @@ static int selinux_set_mapping(struct policydb *pol,
p_out->value = string_to_security_class(pol, p_in->name);
if (!p_out->value) {
- printk(KERN_INFO
- "SELinux: Class %s not defined in policy.\n",
+ pr_info("SELinux: Class %s not defined in policy.\n",
p_in->name);
if (pol->reject_unknown)
goto err;
@@ -156,8 +155,7 @@ static int selinux_set_mapping(struct policydb *pol,
p_out->perms[k] = string_to_av_perm(pol, p_out->value,
p_in->perms[k]);
if (!p_out->perms[k]) {
- printk(KERN_INFO
- "SELinux: Permission %s in class %s not
defined in policy.\n",
+ pr_info("SELinux: Permission %s in class %s
not defined in policy.\n",
p_in->perms[k], p_in->name);
if (pol->reject_unknown)
goto err;
@@ -170,7 +168,7 @@ static int selinux_set_mapping(struct policydb *pol,
}
if (print_unknown_handle)
- printk(KERN_INFO "SELinux: the above unknown classes and
permissions will be %s\n",
+ pr_info("SELinux: the above unknown classes and permissions
will be %s\n",
pol->allow_unknown ? "allowed" : "denied");
out_map->size = i;
@@ -644,7 +642,7 @@ static void context_struct_compute_av(struct policydb
*policydb,
if (unlikely(!tclass || tclass > policydb->p_classes.nprim)) {
if (printk_ratelimit())
- printk(KERN_WARNING "SELinux: Invalid class %hu\n",
tclass);
+ pr_warn("SELinux: Invalid class %hu\n", tclass);
return;
}
@@ -793,7 +791,7 @@ static int security_compute_validatetrans(struct
selinux_state *state,
ocontext = sidtab_search(sidtab, oldsid);
if (!ocontext) {
- printk(KERN_ERR "SELinux: %s: unrecognized SID %d\n",
+ pr_err("SELinux: %s: unrecognized SID %d\n",
__func__, oldsid);
rc = -EINVAL;
goto out;
@@ -801,7 +799,7 @@ static int security_compute_validatetrans(struct
selinux_state *state,
ncontext = sidtab_search(sidtab, newsid);
if (!ncontext) {
- printk(KERN_ERR "SELinux: %s: unrecognized SID %d\n",
+ pr_err("SELinux: %s: unrecognized SID %d\n",
__func__, newsid);
rc = -EINVAL;
goto out;
@@ -809,7 +807,7 @@ static int security_compute_validatetrans(struct
selinux_state *state,
tcontext = sidtab_search(sidtab, tasksid);
if (!tcontext) {
- printk(KERN_ERR "SELinux: %s: unrecognized SID %d\n",
+ pr_err("SELinux: %s: unrecognized SID %d\n",
__func__, tasksid);
rc = -EINVAL;
goto out;
@@ -883,7 +881,7 @@ int security_bounded_transition(struct selinux_state *state,
rc = -EINVAL;
old_context = sidtab_search(sidtab, old_sid);
if (!old_context) {
- printk(KERN_ERR "SELinux: %s: unrecognized SID %u\n",
+ pr_err("SELinux: %s: unrecognized SID %u\n",
__func__, old_sid);
goto out;
}
@@ -891,7 +889,7 @@ int security_bounded_transition(struct selinux_state *state,
rc = -EINVAL;
new_context = sidtab_search(sidtab, new_sid);
if (!new_context) {
- printk(KERN_ERR "SELinux: %s: unrecognized SID %u\n",
+ pr_err("SELinux: %s: unrecognized SID %u\n",
__func__, new_sid);
goto out;
}
@@ -1040,14 +1038,14 @@ void security_compute_xperms_decision(struct
selinux_state *state,
scontext = sidtab_search(sidtab, ssid);
if (!scontext) {
- printk(KERN_ERR "SELinux: %s: unrecognized SID %d\n",
+ pr_err("SELinux: %s: unrecognized S
[PATCH 10/13] selinux: Cleanup printk logging in netport
Replace printk with pr_* to avoid checkpatch warnings.
Signed-off-by: Peter Enderborg
---
security/selinux/netport.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/security/selinux/netport.c b/security/selinux/netport.c
index 9ed4c5064a5e..7a141cadbffc 100644
--- a/security/selinux/netport.c
+++ b/security/selinux/netport.c
@@ -173,9 +173,8 @@ static int sel_netport_sid_slow(u8 protocol, u16 pnum, u32
*sid)
out:
spin_unlock_bh(_netport_lock);
if (unlikely(ret)) {
- printk(KERN_WARNING
- "SELinux: failure in sel_netport_sid_slow(),"
- " unable to determine network port label\n");
+ pr_warn("SELinux: failure in %s(), unable to determine network
port label\n",
+ __func__);
kfree(new);
}
return ret;
--
2.15.1
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
[PATCH 13/13] selinux: Cleanup printk logging in netnode
Replace printk with pr_* to avoid checkpatch warnings.
Signed-off-by: Peter Enderborg
---
security/selinux/netnode.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/security/selinux/netnode.c b/security/selinux/netnode.c
index 6dd89b89bc1f..afa0d432436b 100644
--- a/security/selinux/netnode.c
+++ b/security/selinux/netnode.c
@@ -238,9 +238,8 @@ static int sel_netnode_sid_slow(void *addr, u16 family, u32
*sid)
out:
spin_unlock_bh(_netnode_lock);
if (unlikely(ret)) {
- printk(KERN_WARNING
- "SELinux: failure in sel_netnode_sid_slow(),"
- " unable to determine network node label\n");
+ pr_warn("SELinux: failure in %s(), unable to determine network
node label\n",
+ __func__);
kfree(new);
}
return ret;
--
2.15.1
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
[PATCH 12/13] selinux: Cleanup printk logging in avc
Replace printk with pr_* to avoid checkpatch warnings.
Signed-off-by: Peter Enderborg
---
security/selinux/avc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/selinux/avc.c b/security/selinux/avc.c
index f3aedf077509..635e5c1e3e48 100644
--- a/security/selinux/avc.c
+++ b/security/selinux/avc.c
@@ -650,7 +650,7 @@ static int avc_latest_notif_update(struct selinux_avc *avc,
spin_lock_irqsave(_lock, flag);
if (is_insert) {
if (seqno < avc->avc_cache.latest_notif) {
- printk(KERN_WARNING "SELinux: avc: seqno %d <
latest_notif %d\n",
+ pr_warn("SELinux: avc: seqno %d < latest_notif %d\n",
seqno, avc->avc_cache.latest_notif);
ret = -EAGAIN;
}
--
2.15.1
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
[PATCH 05/13] selinux: Cleanup printk logging in avtab
Replace printk with pr_* to avoid checkpatch warnings.
Signed-off-by: Peter Enderborg
---
security/selinux/ss/avtab.c | 51 +++--
1 file changed, 26 insertions(+), 25 deletions(-)
diff --git a/security/selinux/ss/avtab.c b/security/selinux/ss/avtab.c
index a2c9148b0662..c0417cf17fee 100644
--- a/security/selinux/ss/avtab.c
+++ b/security/selinux/ss/avtab.c
@@ -338,7 +338,7 @@ int avtab_alloc(struct avtab *h, u32 nrules)
h->nel = 0;
h->nslot = nslot;
h->mask = mask;
- printk(KERN_DEBUG "SELinux: %d avtab hash slots, %d rules.\n",
+ pr_debug("SELinux: %d avtab hash slots, %d rules.\n",
h->nslot, nrules);
return 0;
}
@@ -368,7 +368,7 @@ void avtab_hash_eval(struct avtab *h, char *tag)
}
}
- printk(KERN_DEBUG "SELinux: %s: %d entries and %d/%d buckets used, "
+ pr_debug("SELinux: %s: %d entries and %d/%d buckets used, "
"longest chain length %d sum of chain length^2 %llu\n",
tag, h->nel, slots_used, h->nslot, max_chain_len,
chain2_len_sum);
@@ -407,18 +407,18 @@ int avtab_read_item(struct avtab *a, void *fp, struct
policydb *pol,
if (vers < POLICYDB_VERSION_AVTAB) {
rc = next_entry(buf32, fp, sizeof(u32));
if (rc) {
- printk(KERN_ERR "SELinux: avtab: truncated entry\n");
+ pr_err("SELinux: avtab: truncated entry\n");
return rc;
}
items2 = le32_to_cpu(buf32[0]);
if (items2 > ARRAY_SIZE(buf32)) {
- printk(KERN_ERR "SELinux: avtab: entry overflow\n");
+ pr_err("SELinux: avtab: entry overflow\n");
return -EINVAL;
}
rc = next_entry(buf32, fp, sizeof(u32)*items2);
if (rc) {
- printk(KERN_ERR "SELinux: avtab: truncated entry\n");
+ pr_err("SELinux: avtab: truncated entry\n");
return rc;
}
items = 0;
@@ -426,19 +426,19 @@ int avtab_read_item(struct avtab *a, void *fp, struct
policydb *pol,
val = le32_to_cpu(buf32[items++]);
key.source_type = (u16)val;
if (key.source_type != val) {
- printk(KERN_ERR "SELinux: avtab: truncated source
type\n");
+ pr_err("SELinux: avtab: truncated source type\n");
return -EINVAL;
}
val = le32_to_cpu(buf32[items++]);
key.target_type = (u16)val;
if (key.target_type != val) {
- printk(KERN_ERR "SELinux: avtab: truncated target
type\n");
+ pr_err("SELinux: avtab: truncated target type\n");
return -EINVAL;
}
val = le32_to_cpu(buf32[items++]);
key.target_class = (u16)val;
if (key.target_class != val) {
- printk(KERN_ERR "SELinux: avtab: truncated target
class\n");
+ pr_err("SELinux: avtab: truncated target class\n");
return -EINVAL;
}
@@ -446,16 +446,16 @@ int avtab_read_item(struct avtab *a, void *fp, struct
policydb *pol,
enabled = (val & AVTAB_ENABLED_OLD) ? AVTAB_ENABLED : 0;
if (!(val & (AVTAB_AV | AVTAB_TYPE))) {
- printk(KERN_ERR "SELinux: avtab: null entry\n");
+ pr_err("SELinux: avtab: null entry\n");
return -EINVAL;
}
if ((val & AVTAB_AV) &&
(val & AVTAB_TYPE)) {
- printk(KERN_ERR "SELinux: avtab: entry has both access
vectors and types\n");
+ pr_err("SELinux: avtab: entry has both access vectors
and types\n");
return -EINVAL;
}
if (val & AVTAB_XPERMS) {
- printk(KERN_ERR "SELinux: avtab: entry has extended
permissions\n");
+ pr_err("SELinux: avtab: entry has extended
permissions\n");
return -EINVAL;
}
@@ -470,7 +470,8 @@ int avtab_read_item(struct avtab *a, void *fp, struct
policydb *pol,
}
if (items != items2) {
- printk(KERN_ERR "SELinux: avtab: entry only had %d
items, expected %d\n", items2, items);
+ pr_err("SELinux: avta
[PATCH 04/13] selinux: Cleanup printk logging in hooks
Replace printk with pr_* to avoid checkpatch warnings.
Signed-off-by: Peter Enderborg
---
security/selinux/hooks.c | 68 +++-
1 file changed, 33 insertions(+), 35 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 4cafe6a19167..3ab9687ac4c8 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -531,7 +531,7 @@ static int sb_finish_set_opts(struct super_block *sb)
the first boot of the SELinux kernel before we have
assigned xattr values to the filesystem. */
if (!(root_inode->i_opflags & IOP_XATTR)) {
- printk(KERN_WARNING "SELinux: (dev %s, type %s) has no "
+ pr_warn("SELinux: (dev %s, type %s) has no "
"xattr support\n", sb->s_id, sb->s_type->name);
rc = -EOPNOTSUPP;
goto out;
@@ -540,11 +540,11 @@ static int sb_finish_set_opts(struct super_block *sb)
rc = __vfs_getxattr(root, root_inode, XATTR_NAME_SELINUX, NULL,
0);
if (rc < 0 && rc != -ENODATA) {
if (rc == -EOPNOTSUPP)
- printk(KERN_WARNING "SELinux: (dev %s, type "
+ pr_warn("SELinux: (dev %s, type "
"%s) has no security xattr handler\n",
sb->s_id, sb->s_type->name);
else
- printk(KERN_WARNING "SELinux: (dev %s, type "
+ pr_warn("SELinux: (dev %s, type "
"%s) getxattr errno %d\n", sb->s_id,
sb->s_type->name, -rc);
goto out;
@@ -743,7 +743,7 @@ static int selinux_set_mnt_opts(struct super_block *sb,
goto out;
}
rc = -EINVAL;
- printk(KERN_WARNING "SELinux: Unable to set superblock options "
+ pr_warn("SELinux: Unable to set superblock options "
"before the security server is initialized\n");
goto out;
}
@@ -785,7 +785,7 @@ static int selinux_set_mnt_opts(struct super_block *sb,
mount_options[i], ,
GFP_KERNEL);
if (rc) {
- printk(KERN_WARNING "SELinux:
security_context_str_to_sid"
+ pr_warn("SELinux: security_context_str_to_sid"
"(%s) failed for (dev %s, type %s) errno=%d\n",
mount_options[i], sb->s_id, name, rc);
goto out;
@@ -861,8 +861,7 @@ static int selinux_set_mnt_opts(struct super_block *sb,
*/
rc = security_fs_use(_state, sb);
if (rc) {
- printk(KERN_WARNING
- "%s: security_fs_use(%s) returned %d\n",
+ pr_warn("%s: security_fs_use(%s) returned %d\n",
__func__, sb->s_type->name, rc);
goto out;
}
@@ -948,7 +947,7 @@ static int selinux_set_mnt_opts(struct super_block *sb,
if (sbsec->behavior != SECURITY_FS_USE_XATTR &&
sbsec->behavior != SECURITY_FS_USE_NATIVE) {
rc = -EINVAL;
- printk(KERN_WARNING "SELinux: defcontext option is "
+ pr_warn("SELinux: defcontext option is "
"invalid for this filesystem type\n");
goto out;
}
@@ -970,7 +969,7 @@ static int selinux_set_mnt_opts(struct super_block *sb,
return rc;
out_double_mount:
rc = -EINVAL;
- printk(KERN_WARNING "SELinux: mount invalid. Same superblock,
different "
+ pr_warn("SELinux: mount invalid. Same superblock, different "
"security settings for (dev %s, type %s)\n", sb->s_id, name);
goto out;
}
@@ -999,7 +998,7 @@ static int selinux_cmp_sb_context(const struct super_block
*oldsb,
}
return 0;
mismatch:
- printk(KERN_WARNING "SELinux: mount invalid. Same superblock, "
+ pr_warn("SELinux: mount invalid. Same superblock, "
"different security settings for (dev %s, "
"type %s)\n", newsb->s_id, newsb->s_type->name);
[PATCH 09/13] selinux: Cleanup printk logging in sidtab
Replace printk with pr_* to avoid checkpatch warnings.
Signed-off-by: Peter Enderborg
---
security/selinux/ss/sidtab.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/security/selinux/ss/sidtab.c b/security/selinux/ss/sidtab.c
index 5be31b7af225..fd75a12fa8fc 100644
--- a/security/selinux/ss/sidtab.c
+++ b/security/selinux/ss/sidtab.c
@@ -214,8 +214,7 @@ int sidtab_context_to_sid(struct sidtab *s,
}
sid = s->next_sid++;
if (context->len)
- printk(KERN_INFO
- "SELinux: Context %s is not valid (left unmapped).\n",
+ pr_info("SELinux: Context %s is not valid (left
unmapped).\n",
context->str);
ret = sidtab_insert(s, sid, context);
if (ret)
@@ -253,7 +252,7 @@ void sidtab_hash_eval(struct sidtab *h, char *tag)
}
}
- printk(KERN_DEBUG "%s: %d entries and %d/%d buckets used, longest "
+ pr_debug("%s: %d entries and %d/%d buckets used, longest "
"chain length %d\n", tag, h->nel, slots_used, SIDTAB_SIZE,
max_chain_len);
}
--
2.15.1
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: [PATCH V3 0/5] selinux:Significant reduce of preempt_disable holds
On 05/31/2018 02:42 PM, Stephen Smalley wrote: > On 05/31/2018 05:04 AM, peter enderborg wrote: >> On 05/30/2018 10:34 PM, Stephen Smalley wrote: >>> On 05/30/2018 10:10 AM, Peter Enderborg wrote: >>>> The boolean change becomes a lot more heavy with this patch, >>>> but it is a very rare usage in compare with read only operations. >>>> The lock held during a policydb_copy is about 1ms on a XEON. >>> This has a very substantial performance impact on setsebool, e.g. time >>> setsebool httpd_can_sendmail=1. >>> That's because you are doing a full >>> vmalloc();policydb_write();policydb_read();vfree() sequence on it. >>> In comparison, KaiGai's old attempt to replace the policy rwlock with RCU >>> only duplicated the conditional policydb state (via a cond_policydb_dup) >>> that he introduced. Is there a reason you couldn't use that approach? >> That one did not make it, so I went for a other path. Make it simple, using >> the same serialisation that exist. That also make it easier to maintain. >> We do not use the booleans in android since they are not allowed so im not >> aware of any use case where this administrative function are >> used in such frequent manner that it would have an impact. And it must be >> some other large overhead with interprocess communication and >> a multiple writes to sysfs during a boolean settings? However my concern >> is/was memory pressure, setting booleans will generate pressure >> with lot of atomic allocation and large vmallocs. > Yes, that is also a concern. I would prefer to only duplicate the > conditional policydb state as in KaiGai's patch. > Keeping temporary setting of booleans lightweight is desirable for other use > cases than Android. > > I'm also concerned by the implications of switching all of the allocations to > atomic. KaiGai's patch did not take that approach either, and it obviously > could make policy reload more prone to transient failures. It maybe not needed atomic at the time. But the duplication holds a rcu_read_lock so it need to be atomic now. > > But my goal is the fast path for real time critical functions such as audio, > and it will be a cost for >> administrative tasks. On the xeon it takes about ~98 ms to run the >> security_set_bools compared to about ~8 ms without the overhead >> of copying the policydb. About ~6 ms is rcu sync and ~8 ms is the same as >> the original update of selinux statuses, and about ~25 ms >> is policydb_destroy() of the old copy. > > > ___ Selinux mailing list Selinux@tycho.nsa.gov To unsubscribe, send email to selinux-le...@tycho.nsa.gov. To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: [PATCH V3 0/5] selinux:Significant reduce of preempt_disable holds
On 05/31/2018 02:42 PM, Stephen Smalley wrote: > On 05/31/2018 05:04 AM, peter enderborg wrote: >> On 05/30/2018 10:34 PM, Stephen Smalley wrote: >>> On 05/30/2018 10:10 AM, Peter Enderborg wrote: >>>> The boolean change becomes a lot more heavy with this patch, >>>> but it is a very rare usage in compare with read only operations. >>>> The lock held during a policydb_copy is about 1ms on a XEON. >>> This has a very substantial performance impact on setsebool, e.g. time >>> setsebool httpd_can_sendmail=1. >>> That's because you are doing a full >>> vmalloc();policydb_write();policydb_read();vfree() sequence on it. >>> In comparison, KaiGai's old attempt to replace the policy rwlock with RCU >>> only duplicated the conditional policydb state (via a cond_policydb_dup) >>> that he introduced. Is there a reason you couldn't use that approach? >> That one did not make it, so I went for a other path. Make it simple, using >> the same serialisation that exist. That also make it easier to maintain. >> We do not use the booleans in android since they are not allowed so im not >> aware of any use case where this administrative function are >> used in such frequent manner that it would have an impact. And it must be >> some other large overhead with interprocess communication and >> a multiple writes to sysfs during a boolean settings? However my concern >> is/was memory pressure, setting booleans will generate pressure >> with lot of atomic allocation and large vmallocs. > Yes, that is also a concern. I would prefer to only duplicate the > conditional policydb state as in KaiGai's patch. > Keeping temporary setting of booleans lightweight is desirable for other use > cases than Android. > > I'm also concerned by the implications of switching all of the allocations to > atomic. KaiGai's patch did not take that approach either, and it obviously > could make policy reload more prone to transient failures. On the version 2 of the patchset you pointed out that I did a shallow copy, so I did a "deap" copy. As I see it the KaiGai cond_policydb_dup also do a shallow copy. You dont happend to know exactly why KaiGai's patch never was accepted? > But my goal is the fast path for real time critical functions such as audio, > and it will be a cost for >> administrative tasks. On the xeon it takes about ~98 ms to run the >> security_set_bools compared to about ~8 ms without the overhead >> of copying the policydb. About ~6 ms is rcu sync and ~8 ms is the same as >> the original update of selinux statuses, and about ~25 ms >> is policydb_destroy() of the old copy. > > > ___ Selinux mailing list Selinux@tycho.nsa.gov To unsubscribe, send email to selinux-le...@tycho.nsa.gov. To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: [PATCH V3 3/5 selinux-next] selinux: sidtab_clone switch to use rwlock.
On 05/30/2018 11:22 PM, J Freyensee wrote:
>
>> +int sidtab_clone(struct sidtab *s, struct sidtab *d)
>> +{
>> + int i, rc = 0;
> If s or d are NULL (see if() below), why would we want rc, the return value,
> to be 0? How about defaulting rc to an error value (-EINVAL)?
Oops! Thanks, will fix in next set.
>> + struct sidtab_node *cur;
>> +
>> + if (!s || !d)
>> + goto errout;
>> +
>> + read_lock(>lock);
>> + for (i = 0; i < SIDTAB_SIZE; i++) {
>> + cur = s->htable[i];
>> + while (cur) {
>> + if (cur->sid > SECINITSID_NUM)
>> + rc = sidtab_insert(d, cur->sid, >context);
>> + if (rc)
>> + goto out;
>> + cur = cur->next;
>> + }
>> + }
>> +out:
>> + read_unlock(>lock);
>> +errout:
>> + return rc;
>> +}
>>
> Thanks,
> Jay
>
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
Re: [PATCH V3 0/5] selinux:Significant reduce of preempt_disable holds
On 05/30/2018 10:34 PM, Stephen Smalley wrote: > On 05/30/2018 10:10 AM, Peter Enderborg wrote: >> The boolean change becomes a lot more heavy with this patch, >> but it is a very rare usage in compare with read only operations. >> The lock held during a policydb_copy is about 1ms on a XEON. > This has a very substantial performance impact on setsebool, e.g. time > setsebool httpd_can_sendmail=1. > That's because you are doing a full > vmalloc();policydb_write();policydb_read();vfree() sequence on it. > In comparison, KaiGai's old attempt to replace the policy rwlock with RCU > only duplicated the conditional policydb state (via a cond_policydb_dup) that > he introduced. Is there a reason you couldn't use that approach? That one did not make it, so I went for a other path. Make it simple, using the same serialisation that exist. That also make it easier to maintain. We do not use the booleans in android since they are not allowed so im not aware of any use case where this administrative function are used in such frequent manner that it would have an impact. And it must be some other large overhead with interprocess communication and a multiple writes to sysfs during a boolean settings? However my concern is/was memory pressure, setting booleans will generate pressure with lot of atomic allocation and large vmallocs. But my goal is the fast path for real time critical functions such as audio, and it will be a cost for administrative tasks. On the xeon it takes about ~98 ms to run the security_set_bools compared to about ~8 ms without the overhead of copying the policydb. About ~6 ms is rcu sync and ~8 ms is the same as the original update of selinux statuses, and about ~25 ms is policydb_destroy() of the old copy. > ___ Selinux mailing list Selinux@tycho.nsa.gov To unsubscribe, send email to selinux-le...@tycho.nsa.gov. To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
[PATCH V3 2/5 selinux-next] selinux: Introduce selinux_ruleset struct
This is a preparation for moving locking to rcu type.
We move policydb, sidtab and map to this structure which
is dynamic allocated. To help out the handlig a policydb_copy
are added. It is intended to be used in atomic context within
a rcu lock, so there are help functions that do vmalloc
allocation that are intended to be on the outside of the lock.
hastab_insert had a cond_sched call that is removed. When switched
to rcu lock the lock can be preempted.
Signed-off-by: Peter Enderborg
---
security/selinux/ss/hashtab.c | 1 -
security/selinux/ss/policydb.c | 48 +++
security/selinux/ss/policydb.h | 6 +-
security/selinux/ss/services.c | 292 +++--
security/selinux/ss/services.h | 12 +-
5 files changed, 226 insertions(+), 133 deletions(-)
diff --git a/security/selinux/ss/hashtab.c b/security/selinux/ss/hashtab.c
index 0944b1f8060e..967b6e3d25c6 100644
--- a/security/selinux/ss/hashtab.c
+++ b/security/selinux/ss/hashtab.c
@@ -44,7 +44,6 @@ int hashtab_insert(struct hashtab *h, void *key, void *datum)
u32 hvalue;
struct hashtab_node *prev, *cur, *newnode;
- cond_resched();
if (!h || h->nel == HASHTAB_MAX_NODES)
return -EINVAL;
diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index 2a0e21d8c275..93d134d057a7 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -3535,3 +3535,51 @@ int policydb_write(struct policydb *p, void *fp)
return 0;
}
+
+int policydb_flattened_alloc(struct policydb *db, void **tmpbuf, size_t *size)
+{
+ int rc = 0;
+
+ *size = db->len;
+ *tmpbuf = vmalloc(*size);
+
+ if (!*tmpbuf) {
+ rc = -ENOMEM;
+ printk(KERN_ERR "SELinux: vmalloc failed for %ld\n", *size);
+ }
+ return rc;
+}
+
+int policydb_flattened_free(void *tmpbuf)
+{
+ vfree(tmpbuf);
+ return 0;
+}
+
+int policydb_copy(struct policydb *olddb, struct policydb *newdb,
+ void **tmpstorage, size_t size)
+{
+ struct policy_file fp;
+ void *data = *tmpstorage;
+ int rc;
+
+ if (size != olddb->len) {
+ rc = -EAGAIN;
+ goto out;
+ }
+ fp.data = data;
+ fp.len = size;
+ rc = policydb_write(olddb, );
+ if (rc)
+ goto out;
+
+ fp.len = size;
+ fp.data = data;
+ rc = policydb_read(newdb, );
+ if (rc)
+ goto out;
+
+ newdb->len = size;
+out:
+ return rc;
+}
diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h
index 215f8f30ac5a..3e2f86b5b674 100644
--- a/security/selinux/ss/policydb.h
+++ b/security/selinux/ss/policydb.h
@@ -320,7 +320,11 @@ extern int policydb_type_isvalid(struct policydb *p,
unsigned int type);
extern int policydb_role_isvalid(struct policydb *p, unsigned int role);
extern int policydb_read(struct policydb *p, void *fp);
extern int policydb_write(struct policydb *p, void *fp);
-
+extern int policydb_copy(struct policydb *olddb, struct policydb *newdb,
+void **tmpstorage, size_t size);
+extern int policydb_flattened_alloc(struct policydb *db,
+ void **tmpbuf, size_t *size);
+extern int policydb_flattened_free(void *tmpbuf);
#define PERM_SYMTAB_SIZE 32
#define POLICYDB_CONFIG_MLS1
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 8057e19dc15f..4f3ce389084c 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -86,6 +86,10 @@ void selinux_ss_init(struct selinux_ss **ss)
{
rwlock_init(_ss.policy_rwlock);
mutex_init(_ss.status_lock);
+ selinux_ss.active_set = kzalloc(sizeof(struct selinux_ruleset),
+ GFP_KERNEL);
+ selinux_ss.active_set->sidtab = kzalloc(sizeof(struct sidtab),
+ GFP_KERNEL);
*ss = _ss;
}
@@ -249,7 +253,7 @@ static void map_decision(struct selinux_map *map,
int security_mls_enabled(struct selinux_state *state)
{
- struct policydb *p = >ss->policydb;
+ struct policydb *p = >ss->active_set->policydb;
return p->mls_enabled;
}
@@ -733,7 +737,7 @@ static int security_validtrans_handle_fail(struct
selinux_state *state,
struct context *tcontext,
u16 tclass)
{
- struct policydb *p = >ss->policydb;
+ struct policydb *p = >ss->active_set->policydb;
char *o = NULL, *n = NULL, *t = NULL;
u32 olen, nlen, tlen;
@@ -777,11 +781,11 @@ static int security_compute_validatetrans(struct
selinux_state *state,
read_lock(>ss->policy_rwlock);
- policydb = >ss->policydb;
- sidtab = >ss->sidtab;
+ policydb
[PATCH V3 1/5 selinux-next] selinux: Make allocation atomic in policydb objects functions.
From: peter
As preparation for RCU the allocation need to be atomic,
there is a lot of them so they do in this patch.
Signed-off-by: Peter Enderborg
---
security/selinux/ss/avtab.c | 8 +--
security/selinux/ss/conditional.c | 14 ++---
security/selinux/ss/ebitmap.c | 3 +-
security/selinux/ss/hashtab.c | 6 +--
security/selinux/ss/policydb.c| 104 +++---
5 files changed, 69 insertions(+), 66 deletions(-)
diff --git a/security/selinux/ss/avtab.c b/security/selinux/ss/avtab.c
index a2c9148b0662..1114a308aa94 100644
--- a/security/selinux/ss/avtab.c
+++ b/security/selinux/ss/avtab.c
@@ -72,13 +72,13 @@ avtab_insert_node(struct avtab *h, int hvalue,
{
struct avtab_node *newnode;
struct avtab_extended_perms *xperms;
- newnode = kmem_cache_zalloc(avtab_node_cachep, GFP_KERNEL);
+ newnode = kmem_cache_zalloc(avtab_node_cachep, GFP_ATOMIC);
if (newnode == NULL)
return NULL;
newnode->key = *key;
if (key->specified & AVTAB_XPERMS) {
- xperms = kmem_cache_zalloc(avtab_xperms_cachep, GFP_KERNEL);
+ xperms = kmem_cache_zalloc(avtab_xperms_cachep, GFP_ATOMIC);
if (xperms == NULL) {
kmem_cache_free(avtab_node_cachep, newnode);
return NULL;
@@ -95,7 +95,7 @@ avtab_insert_node(struct avtab *h, int hvalue,
} else {
newnode->next = flex_array_get_ptr(h->htable, hvalue);
if (flex_array_put_ptr(h->htable, hvalue, newnode,
- GFP_KERNEL|__GFP_ZERO)) {
+ GFP_ATOMIC|__GFP_ZERO)) {
kmem_cache_free(avtab_node_cachep, newnode);
return NULL;
}
@@ -330,7 +330,7 @@ int avtab_alloc(struct avtab *h, u32 nrules)
mask = nslot - 1;
h->htable = flex_array_alloc(sizeof(struct avtab_node *), nslot,
-GFP_KERNEL | __GFP_ZERO);
+GFP_ATOMIC | __GFP_ZERO);
if (!h->htable)
return -ENOMEM;
diff --git a/security/selinux/ss/conditional.c
b/security/selinux/ss/conditional.c
index c91543a617ac..a09c8a8e9472 100644
--- a/security/selinux/ss/conditional.c
+++ b/security/selinux/ss/conditional.c
@@ -178,7 +178,7 @@ int cond_init_bool_indexes(struct policydb *p)
kfree(p->bool_val_to_struct);
p->bool_val_to_struct = kmalloc_array(p->p_bools.nprim,
sizeof(*p->bool_val_to_struct),
- GFP_KERNEL);
+ GFP_ATOMIC);
if (!p->bool_val_to_struct)
return -ENOMEM;
return 0;
@@ -205,7 +205,7 @@ int cond_index_bool(void *key, void *datum, void *datap)
fa = p->sym_val_to_name[SYM_BOOLS];
if (flex_array_put_ptr(fa, booldatum->value - 1, key,
- GFP_KERNEL | __GFP_ZERO))
+ GFP_ATOMIC | __GFP_ZERO))
BUG();
p->bool_val_to_struct[booldatum->value - 1] = booldatum;
@@ -227,7 +227,7 @@ int cond_read_bool(struct policydb *p, struct hashtab *h,
void *fp)
u32 len;
int rc;
- booldatum = kzalloc(sizeof(*booldatum), GFP_KERNEL);
+ booldatum = kzalloc(sizeof(*booldatum), GFP_ATOMIC);
if (!booldatum)
return -ENOMEM;
@@ -247,7 +247,7 @@ int cond_read_bool(struct policydb *p, struct hashtab *h,
void *fp)
goto err;
rc = -ENOMEM;
- key = kmalloc(len + 1, GFP_KERNEL);
+ key = kmalloc(len + 1, GFP_ATOMIC);
if (!key)
goto err;
rc = next_entry(key, fp, len);
@@ -332,7 +332,7 @@ static int cond_insertf(struct avtab *a, struct avtab_key
*k, struct avtab_datum
goto err;
}
- list = kzalloc(sizeof(*list), GFP_KERNEL);
+ list = kzalloc(sizeof(*list), GFP_ATOMIC);
if (!list) {
rc = -ENOMEM;
goto err;
@@ -420,7 +420,7 @@ static int cond_read_node(struct policydb *p, struct
cond_node *node, void *fp)
goto err;
rc = -ENOMEM;
- expr = kzalloc(sizeof(*expr), GFP_KERNEL);
+ expr = kzalloc(sizeof(*expr), GFP_ATOMIC);
if (!expr)
goto err;
@@ -471,7 +471,7 @@ int cond_read_list(struct policydb *p, void *fp)
for (i = 0; i < len; i++) {
rc = -ENOMEM;
- node = kzalloc(sizeof(*node), GFP_KERNEL);
+ node = kzalloc(sizeof(*node), GFP_ATOMIC);
if (!node)
goto err;
diff --git a/security/selinux/ss/ebitmap.c b/security/selinux/ss/ebitmap.c
index 5ae8c61b75bf..a49fab
[PATCH V3 0/5] selinux:Significant reduce of preempt_disable holds
Holding the preempt_disable is very bad for low latency tasks such as audio and therefore we need to break out the rule-set dependent part from this disable. By using a RCU instead of rwlock we have an efficient locking and less preemption interference. Selinux uses a lot of read_locks. This patch replaces the rwlock with RCU that does not hold preempt_disable. Intel Xeon W3520 2.67 Ghz running FC27 with 4.15.0-rc9git (+measurement) I get preempt_disable of about 1.2ms in security_compute_av(). With the patch I get 960us as the longest security_compute_av() without preempt disabeld. There are very much noise in the measurement but it is not likely a degrade. And the preempt_disable times is also very dependent on the selinux rule-set. In security_get_user_sids() we have two nested for-loops and the inner part calls sittab_context_to_sid() that calls sidtab_search_context() that has a for loop() over a while() where the loops is dependent on the rules. On the test system the average lookup time is 60us and does not change with the introduced RCU usage. The boolean change becomes a lot more heavy with this patch, but it is a very rare usage in compare with read only operations. The lock held during a policydb_copy is about 1ms on a XEON. To use RCU the structure of policydb has to be accesses through a pointer. We need 5 patches to get there. [PATCH V3 1/5 selinux-next] selinux: Make allocation atomic in policydb objects functions. This patch change the allocation for policydb objects. They are in its own patch to make the complicated part easier to read. [PATCH V3 2/5 selinux-next] selinux: Introduce selinux_ruleset struct This makes the access for the rule evaluation going though a single pointer. [PATCH V3 3/5 selinux-next] selinux: sidtab_clone switch to use rwlock. We need to make sidtabs copys so this patch change the locks to a rwlock and create a copy function. [PATCH V3 4/5 selinux-next] selinux: seqno separation This patch adds separation of the read and write and uses the pointer to switch rule set. It uses seqno for error handling since there are a possibility to have multiple access. [PATCH V3 5/5 selinux-next] selinux: Switch to rcu read locks for avc_compute All the preparation is done so this patch do the change of locks to rcu. History: V1 rwsem V2 did not handle all policydb objects, solved with the policydb_copy did not handle sidtab for booleans, I think this one does however shutdown is not used but not removed. ___ Selinux mailing list Selinux@tycho.nsa.gov To unsubscribe, send email to selinux-le...@tycho.nsa.gov. To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
[PATCH V3 5/5 selinux-next] selinux: Switch to rcu read locks for avc_compute
To be able to preempt avc_compute we need preemptible
locks, this patch switch the rwlock reads to rcu_read_lock.
Signed-off-by: Peter Enderborg
---
security/selinux/ss/services.c | 152 +
security/selinux/ss/services.h | 2 +-
2 files changed, 79 insertions(+), 75 deletions(-)
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 954ebe490516..a9aa863c47a3 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -84,7 +84,7 @@ static struct selinux_ss selinux_ss;
void selinux_ss_init(struct selinux_ss **ss)
{
- rwlock_init(_ss.policy_rwlock);
+ spin_lock_init(_ss.policy_lock);
mutex_init(_ss.status_lock);
selinux_ss.active_set = kzalloc(sizeof(struct selinux_ruleset),
GFP_KERNEL);
@@ -779,7 +779,7 @@ static int security_compute_validatetrans(struct
selinux_state *state,
if (!state->initialized)
return 0;
- read_lock(>ss->policy_rwlock);
+ rcu_read_lock();
policydb = >ss->active_set->policydb;
sidtab = state->ss->active_set->sidtab;
@@ -837,7 +837,7 @@ static int security_compute_validatetrans(struct
selinux_state *state,
}
out:
- read_unlock(>ss->policy_rwlock);
+ rcu_read_unlock();
return rc;
}
@@ -879,7 +879,7 @@ int security_bounded_transition(struct selinux_state *state,
if (!state->initialized)
return 0;
- read_lock(>ss->policy_rwlock);
+ rcu_read_lock();
policydb = >ss->active_set->policydb;
sidtab = state->ss->active_set->sidtab;
@@ -944,7 +944,7 @@ int security_bounded_transition(struct selinux_state *state,
kfree(old_name);
}
out:
- read_unlock(>ss->policy_rwlock);
+ rcu_read_unlock();
return rc;
}
@@ -1035,7 +1035,7 @@ void security_compute_xperms_decision(struct
selinux_state *state,
memset(xpermd->auditallow->p, 0, sizeof(xpermd->auditallow->p));
memset(xpermd->dontaudit->p, 0, sizeof(xpermd->dontaudit->p));
- read_lock(>ss->policy_rwlock);
+ rcu_read_lock();
if (!state->initialized)
goto allow;
@@ -1092,7 +1092,7 @@ void security_compute_xperms_decision(struct
selinux_state *state,
}
}
out:
- read_unlock(>ss->policy_rwlock);
+ rcu_read_unlock();
return;
allow:
memset(xpermd->allowed->p, 0xff, sizeof(xpermd->allowed->p));
@@ -1122,7 +1122,7 @@ void security_compute_av(struct selinux_state *state,
u16 tclass;
struct context *scontext = NULL, *tcontext = NULL;
- read_lock(>ss->policy_rwlock);
+ rcu_read_lock();
avd_init(state, avd);
xperms->len = 0;
if (!state->initialized)
@@ -1160,7 +1160,7 @@ void security_compute_av(struct selinux_state *state,
map_decision(>ss->active_set->map, orig_tclass, avd,
policydb->allow_unknown);
out:
- read_unlock(>ss->policy_rwlock);
+ rcu_read_unlock();
return;
allow:
avd->allowed = 0x;
@@ -1177,7 +1177,7 @@ void security_compute_av_user(struct selinux_state *state,
struct sidtab *sidtab;
struct context *scontext = NULL, *tcontext = NULL;
- read_lock(>ss->policy_rwlock);
+ rcu_read_lock();
avd_init(state, avd);
if (!state->initialized)
goto allow;
@@ -1212,7 +1212,7 @@ void security_compute_av_user(struct selinux_state *state,
context_struct_compute_av(policydb, scontext, tcontext, tclass, avd,
NULL);
out:
- read_unlock(>ss->policy_rwlock);
+ rcu_read_unlock();
return;
allow:
avd->allowed = 0x;
@@ -1319,7 +1319,7 @@ static int security_sid_to_context_core(struct
selinux_state *state,
rc = -EINVAL;
goto out;
}
- read_lock(>ss->policy_rwlock);
+ rcu_read_lock();
policydb = >ss->active_set->policydb;
sidtab = state->ss->active_set->sidtab;
if (force)
@@ -1335,7 +1335,7 @@ static int security_sid_to_context_core(struct
selinux_state *state,
rc = context_struct_to_string(policydb, context, scontext,
scontext_len);
out_unlock:
- read_unlock(>ss->policy_rwlock);
+ rcu_read_unlock();
out:
return rc;
@@ -1491,7 +1491,7 @@ static int security_context_to_sid_core(struct
selinux_state *state,
if (!str)
goto out;
}
- read_lock(>ss->policy_rwlock);
+ rcu_read_lock();
policydb = >ss->active_set->policydb;
[PATCH V3 3/5 selinux-next] selinux: sidtab_clone switch to use rwlock.
We need a copy of sidtabs, so change the generic sidtab_clone
as from a function pointer and let it use a read rwlock while
do the clone.
Signed-off-by: Peter Enderborg
---
security/selinux/ss/services.c | 20 +---
security/selinux/ss/sidtab.c | 39 ---
security/selinux/ss/sidtab.h | 3 ++-
3 files changed, 35 insertions(+), 27 deletions(-)
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 4f3ce389084c..2be471d72c85 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -1891,19 +1891,6 @@ int security_change_sid(struct selinux_state *state,
out_sid, false);
}
-/* Clone the SID into the new SID table. */
-static int clone_sid(u32 sid,
-struct context *context,
-void *arg)
-{
- struct sidtab *s = arg;
-
- if (sid > SECINITSID_NUM)
- return sidtab_insert(s, sid, context);
- else
- return 0;
-}
-
static inline int convert_context_handle_invalid_context(
struct selinux_state *state,
struct context *context)
@@ -2199,10 +2186,7 @@ int security_load_policy(struct selinux_state *state,
void *data, size_t len)
goto err;
}
- /* Clone the SID table. */
- sidtab_shutdown(old_set->sidtab);
-
- rc = sidtab_map(old_set->sidtab, clone_sid, next_set->sidtab);
+ rc = sidtab_clone(old_set->sidtab, next_set->sidtab);
if (rc)
goto err;
@@ -2926,8 +2910,6 @@ int security_set_bools(struct selinux_state *state, int
len, int *values)
goto out;
}
- seqno = ++state->ss->latest_granting;
- state->ss->active_set = next_set;
rc = 0;
out:
if (!rc) {
diff --git a/security/selinux/ss/sidtab.c b/security/selinux/ss/sidtab.c
index 5be31b7af225..811503cd7c2b 100644
--- a/security/selinux/ss/sidtab.c
+++ b/security/selinux/ss/sidtab.c
@@ -27,7 +27,7 @@ int sidtab_init(struct sidtab *s)
s->nel = 0;
s->next_sid = 1;
s->shutdown = 0;
- spin_lock_init(>lock);
+ rwlock_init(>lock);
return 0;
}
@@ -116,6 +116,31 @@ struct context *sidtab_search_force(struct sidtab *s, u32
sid)
return sidtab_search_core(s, sid, 1);
}
+int sidtab_clone(struct sidtab *s, struct sidtab *d)
+{
+ int i, rc = 0;
+ struct sidtab_node *cur;
+
+ if (!s || !d)
+ goto errout;
+
+ read_lock(>lock);
+ for (i = 0; i < SIDTAB_SIZE; i++) {
+ cur = s->htable[i];
+ while (cur) {
+ if (cur->sid > SECINITSID_NUM)
+ rc = sidtab_insert(d, cur->sid, >context);
+ if (rc)
+ goto out;
+ cur = cur->next;
+ }
+ }
+out:
+ read_unlock(>lock);
+errout:
+ return rc;
+}
+
int sidtab_map(struct sidtab *s,
int (*apply) (u32 sid,
struct context *context,
@@ -202,7 +227,7 @@ int sidtab_context_to_sid(struct sidtab *s,
if (!sid)
sid = sidtab_search_context(s, context);
if (!sid) {
- spin_lock_irqsave(>lock, flags);
+ write_lock_irqsave(>lock, flags);
/* Rescan now that we hold the lock. */
sid = sidtab_search_context(s, context);
if (sid)
@@ -221,7 +246,7 @@ int sidtab_context_to_sid(struct sidtab *s,
if (ret)
s->next_sid--;
unlock_out:
- spin_unlock_irqrestore(>lock, flags);
+ write_unlock_irqrestore(>lock, flags);
}
if (ret)
@@ -287,21 +312,21 @@ void sidtab_set(struct sidtab *dst, struct sidtab *src)
unsigned long flags;
int i;
- spin_lock_irqsave(>lock, flags);
+ write_lock_irqsave(>lock, flags);
dst->htable = src->htable;
dst->nel = src->nel;
dst->next_sid = src->next_sid;
dst->shutdown = 0;
for (i = 0; i < SIDTAB_CACHE_LEN; i++)
dst->cache[i] = NULL;
- spin_unlock_irqrestore(>lock, flags);
+ write_unlock_irqrestore(>lock, flags);
}
void sidtab_shutdown(struct sidtab *s)
{
unsigned long flags;
- spin_lock_irqsave(>lock, flags);
+ write_lock_irqsave(>lock, flags);
s->shutdown = 1;
- spin_unlock_irqrestore(>lock, flags);
+ write_unlock_irqrestore(>lock, flags);
}
diff --git a/security/selinux/ss/sidtab.h b/security/selinux/ss/sidtab.h
index a1a1d2617b6f..6751f8bcbd66 100644
--- a/security/selinux/ss/sidtab.h
+++ b/security/selinux/ss/sidtab.h
@@ -29,7 +29,7 @@ struct sidtab {
unsigned ch
[PATCH V3 4/5 selinux-next] selinux: seqno separation
This patch separtate the locks for read and write, and
to be sure that they are using the same structure the
seqno is used. If the seqno is changed from the read to
write section the function reportes an eagain error.
Signed-off-by: Peter Enderborg
---
security/selinux/ss/services.c | 143 -
1 file changed, 98 insertions(+), 45 deletions(-)
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 2be471d72c85..954ebe490516 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -2104,6 +2104,9 @@ int security_load_policy(struct selinux_state *state,
void *data, size_t len)
u32 seqno;
int rc = 0;
struct selinux_ruleset *next_set, *old_set;
+ size_t size;
+ void *storage;
+ struct policydb *pdc;
struct policy_file file = { data, len }, *fp =
next_set = kzalloc(sizeof(struct selinux_ruleset), GFP_KERNEL);
@@ -2111,14 +2114,15 @@ int security_load_policy(struct selinux_state *state,
void *data, size_t len)
rc = -ENOMEM;
goto out;
}
+
next_set->sidtab = kzalloc(sizeof(struct sidtab), GFP_KERNEL);
if (!next_set->sidtab) {
rc = -ENOMEM;
- kfree(next_set);
- goto out;
+ goto nexterr;
}
if (!state->initialized) {
+ /* sidtab exist before inititalisation */
old_set = state->ss->active_set;
rc = policydb_read(_set->policydb, fp);
if (rc)
@@ -2152,57 +2156,80 @@ int security_load_policy(struct selinux_state *state,
void *data, size_t len)
kfree(old_set);
goto out;
}
+
+ pdc = kzalloc(sizeof(struct selinux_ruleset), GFP_KERNEL);
+ if (!pdc)
+ goto allocerr;
+
+ rc = policydb_flattened_alloc(>ss->active_set->policydb,
+ , );
+ if (rc)
+ goto pdcerr;
+
+ read_lock(>ss->policy_rwlock);
old_set = state->ss->active_set;
+ rc = policydb_copy(_set->policydb, pdc, , size);
+
+ /* save seq */
+ seqno = state->ss->latest_granting;
+
+ read_unlock(>ss->policy_rwlock);
+
+ policydb_flattened_free(storage);
+
+ if (rc)
+ goto cpyerr;
+
#if 0
sidtab_hash_eval(sidtab, "sids");
#endif
-
rc = policydb_read(_set->policydb, fp);
if (rc)
- goto out;
+ goto cpyerr;
next_set->policydb.len = len;
/* If switching between different policy types, log MLS status */
- if (old_set->policydb.mls_enabled && !next_set->policydb.mls_enabled)
+ if (pdc->mls_enabled && !next_set->policydb.mls_enabled)
printk(KERN_INFO "SELinux: Disabling MLS support...\n");
- else if (!old_set->policydb.mls_enabled
+ else if (!pdc->mls_enabled
&& next_set->policydb.mls_enabled)
printk(KERN_INFO "SELinux: Enabling MLS support...\n");
+
rc = policydb_load_isids(_set->policydb, next_set->sidtab);
if (rc) {
printk(KERN_ERR "SELinux: unable to load the initial SIDs\n");
- policydb_destroy(_set->policydb);
- goto out;
+ goto cpyerr;
}
rc = selinux_set_mapping(_set->policydb, secclass_map, );
if (rc)
- goto err;
+ goto loaderr;
rc = security_preserve_bools(state, _set->policydb);
if (rc) {
printk(KERN_ERR "SELinux: unable to preserve booleans\n");
- goto err;
+ goto maperr;
}
rc = sidtab_clone(old_set->sidtab, next_set->sidtab);
if (rc)
- goto err;
+ goto maperr;
/*
* Convert the internal representations of contexts
* in the new SID table.
*/
args.state = state;
- args.oldp = _set->policydb;
+ args.oldp = pdc;
args.newp = _set->policydb;
+
rc = sidtab_map(next_set->sidtab, convert_context, );
if (rc) {
printk(KERN_ERR "SELinux: unable to convert the internal"
" representation of contexts in the new SID"
" table\n");
- goto err;
+ goto maperr;
}
next_set->map.mapping = newmap.mapping;
@@ -2210,30 +2237,44 @@ int security_load_policy(struct selinux_state *state,
void *data, size_t len)
/* Install the new policydb and SID table. */
write_lock_irq(>ss->policy_rwlock);
- security_load_policycaps(state, _set-&g
Re: [PATCH selinux-next] selinux: Annotate lockdep for services locks
On 02/20/2018 04:58 PM, Stephen Smalley wrote:
> On Tue, 2018-02-20 at 08:59 -0500, Stephen Smalley wrote:
>> On Mon, 2018-02-19 at 16:18 +0100, Peter Enderborg wrote:
>>> From: Peter <peter.enderb...@sony.com>
>>>
>>> The locks are moved to dynamic allocation, we need to
>>> help the lockdep system to classify the locks.
>>> This adds to lockdep annotation for the page mutex and
>>> for the ss lock.
>>>
>>> Signed-off-by: Peter Enderborg <peter.enderb...@sony.com>
>>> ---
>>> This is the rebase of suggested patches from selinuxns tree
>>> and are intended to be applyed on top of:
>>> selinux: wrap global selinux state
>>> from Stephen Smalley
>>>
>>> security/selinux/ss/services.c | 4
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/security/selinux/ss/services.c
>>> b/security/selinux/ss/services.c
>>> index 3698352213d7..a741552e22b5 100644
>>> --- a/security/selinux/ss/services.c
>>> +++ b/security/selinux/ss/services.c
>>> @@ -81,11 +81,15 @@ char
>>> *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX] = {
>>> };
>>>
>>> static struct selinux_ss selinux_ss;
>>> +static struct lock_class_key selinux_ss_class_key;
>>> +static struct lock_class_key selinux_status_class_key;
>>>
>>> void selinux_ss_init(struct selinux_ss **ss)
>>> {
>>> rwlock_init(_ss.policy_rwlock);
>>> + lockdep_set_class(_ss.policy_rwlock,
>>> _ss_class_key);
>>> mutex_init(_ss.status_lock);
>>> + lockdep_set_class(_ss.status_lock,
>>> _status_class_key);
>>> *ss = _ss;
>>> }
>> Pardon my ignorance, but can you explain why we need an explicit call
>> to lockdep_set_class() here? I see it used for e.g. the inode
>> i_lock,
>> but there the class is per-file_system_type. It doesn't seem to be
>> always be used for all locks when they are dynamically initialized or
>> allocated, e.g. get_empty_filp does not call lockdep_set_class() for
>> struct file's f_owner.lock or f_lock even though they are dynamically
>> allocated and initialized. What makes this case different?
> Also, your explanation in the patch description was because the locks
> are moved to dynamic allocation. That was true of the original selinux
> namespace patch. But it isn't true for the wrap global selinux state
> patch; selinux_ss is statically allocated and there is only one
> instance of it in this patch. So do we need this lockdep annotation
> yet?
>
>
I think you are right. I dont get any warnings whey trying to use them, and
lockdep
get a useful name for them.
[PATCH selinux-next] selinux: Annotate lockdep for services locks
From: Peter <peter.enderb...@sony.com>
The locks are moved to dynamic allocation, we need to
help the lockdep system to classify the locks.
This adds to lockdep annotation for the page mutex and
for the ss lock.
Signed-off-by: Peter Enderborg <peter.enderb...@sony.com>
---
This is the rebase of suggested patches from selinuxns tree
and are intended to be applyed on top of:
selinux: wrap global selinux state
from Stephen Smalley
security/selinux/ss/services.c | 4
1 file changed, 4 insertions(+)
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 3698352213d7..a741552e22b5 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -81,11 +81,15 @@ char *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX] = {
};
static struct selinux_ss selinux_ss;
+static struct lock_class_key selinux_ss_class_key;
+static struct lock_class_key selinux_status_class_key;
void selinux_ss_init(struct selinux_ss **ss)
{
rwlock_init(_ss.policy_rwlock);
+ lockdep_set_class(_ss.policy_rwlock, _ss_class_key);
mutex_init(_ss.status_lock);
+ lockdep_set_class(_ss.status_lock, _status_class_key);
*ss = _ss;
}
--
2.14.3
Re: [PATCH v2 4/5] selinux: Use pointer to switch policydb and sidtab
On 01/30/2018 03:37 PM, Stephen Smalley wrote:
> On Fri, 2018-01-26 at 15:32 +0100, peter.enderb...@sony.com wrote:
> goto err;
>
> - rc = security_preserve_bools(newpolicydb);
> + rc = security_preserve_bools(_rcu->policydb);
> if (rc) {
> printk(KERN_ERR "SELinux: unable to preserve
> booleans\n");
> goto err;
> Most of this shouldn't need to be under the read lock.
>
>> @@ -2189,7 +2194,7 @@ int security_load_policy(void *data, size_t
>> len)
>> * in the new SID table.
>> */
>> args.oldp = >policydb;
>> -args.newp = newpolicydb;
>> +args.newp = _rcu->policydb;
>> rc = sidtab_map(, convert_context, );
>> if (rc) {
>> printk(KERN_ERR "SELinux: unable to convert the
>> internal"
>> @@ -2204,8 +2209,9 @@ int security_load_policy(void *data, size_t
>> len)
>>
>> /* Install the new policydb and SID table. */
>> /* next */
>> +security_load_policycaps(_rcu->policydb);
> This cannot be done outside of the write lock; it has to be atomic with
> the policy switch.
Can you please elaborate, does some else write the policydb without a lock?
Is there any other data that is shared? I see this as a private until we switch
the pointer.
>> +read_unlock(_rwlock);
>> write_lock_irq(_rwlock);
>> -memcpy(_rcu->policydb, newpolicydb, sizeof(struct
>> policydb));
>> sidtab_set(_rcu->sidtab, );
>> security_load_policycaps(_rcu->policydb);
>> oldmap = crm->current_mapping;
>> @@ -2213,8 +2219,9 @@ int security_load_policy(void *data, size_t
>> len)
>> next_rcu->current_mapping_size = map_size;
>>
>> seqno = ++latest_granting;
>> -write_unlock_irq(_rwlock);
>> +old_rcu = crm;
>> crm = next_rcu;
>> +write_unlock_irq(_rwlock);
>>
>> /* Free the old policydb and SID table. */
>> policydb_destroy(oldpolicydb);
>> @@ -2226,17 +2233,16 @@ int security_load_policy(void *data, size_t
>> len)
>> selinux_status_update_policyload(seqno);
>> selinux_netlbl_cache_invalidate();
>> selinux_xfrm_notify_policyload();
>> +kfree(oldpolicydb);
>> +kfree(old_rcu);
>>
>> rc = 0;
>> goto out;
>> -
>> err:
>> kfree(map);
>> sidtab_destroy();
>> -policydb_destroy(newpolicydb);
>> -
>> +
Re: [PATCH-selinuxns] selinux: Annotate lockdep for services locks
Resent with sign-off.
If you pick the patches that use the dynamic allocation of locks you can pick
it.
The patches for the annotation does not apply for the selinux-next at the moment
so have to be for selinuxns.
I will send the patches to pauls next tree. Im abit confused on when that is
appropriate. Obviously there will be collisions with the namespace, but
the patches also solves few of my prerequisite topics.
On 02/02/2018 03:10 PM, Stephen Smalley wrote:
> On Fri, 2018-02-02 at 09:05 +0100, Peter Enderborg wrote:
>> The locks are moved to dynamic allocation, we need to
>> help the lockdep system to classify the locks.
>> This adds to lockdep annotation for the page mutex and
>> for the ss lock.
> Thanks, but missing a Signed-off-by: line. Also, just to be clear, you
> shouldn't re-base your work on top of the entire selinuxns branch,
> since I only expect the first few patches to be mergeable in the near
> term.
>
> I also will need to re-base again when the selinux next branch moves to
> something 4.15-based, including fixing up the bpf hooks that were
> introduced there.
>
>> ---
>> security/selinux/ss/services.c | 5 +
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/security/selinux/ss/services.c
>> b/security/selinux/ss/services.c
>> index abc5383..ba463c0 100644
>> --- a/security/selinux/ss/services.c
>> +++ b/security/selinux/ss/services.c
>> @@ -70,6 +70,9 @@
>> #include "ebitmap.h"
>> #include "audit.h"
>>
>> +static struct lock_class_key selinux_ss_class_key;
>> +static struct lock_class_key selinux_status_class_key;
>> +
>> /* Policy capability names */
>> char *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX] = {
>> "network_peer_controls",
>> @@ -88,7 +91,9 @@ int selinux_ss_create(struct selinux_ss **ss)
>> if (!newss)
>> return -ENOMEM;
>> rwlock_init(>policy_rwlock);
>> +lockdep_set_class(>policy_rwlock,
>> _ss_class_key);
>> mutex_init(>status_lock);
>> +lockdep_set_class(>status_lock,
>> _status_class_key);
>> *ss = newss;
>> return 0;
>> }
[PATCH-selinuxns] selinux: Annotate lockdep for services locks
The locks are moved to dynamic allocation, we need to
help the lockdep system to classify the locks.
This adds to lockdep annotation for the page mutex and
for the ss lock.
Signed-off-by: Peter Enderborg <peter.enderb...@sony.com>
---
security/selinux/ss/services.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index abc5383..ba463c0 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -70,6 +70,9 @@
#include "ebitmap.h"
#include "audit.h"
+static struct lock_class_key selinux_ss_class_key;
+static struct lock_class_key selinux_status_class_key;
+
/* Policy capability names */
char *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX] = {
"network_peer_controls",
@@ -88,7 +91,9 @@ int selinux_ss_create(struct selinux_ss **ss)
if (!newss)
return -ENOMEM;
rwlock_init(>policy_rwlock);
+ lockdep_set_class(>policy_rwlock, _ss_class_key);
mutex_init(>status_lock);
+ lockdep_set_class(>status_lock, _status_class_key);
*ss = newss;
return 0;
}
--
2.7.4
[PATCH-selinuxns] selinux: Annotate lockdep for services locks
The locks are moved to dynamic allocation, we need to
help the lockdep system to classify the locks.
This adds to lockdep annotation for the page mutex and
for the ss lock.
---
security/selinux/ss/services.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index abc5383..ba463c0 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -70,6 +70,9 @@
#include "ebitmap.h"
#include "audit.h"
+static struct lock_class_key selinux_ss_class_key;
+static struct lock_class_key selinux_status_class_key;
+
/* Policy capability names */
char *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX] = {
"network_peer_controls",
@@ -88,7 +91,9 @@ int selinux_ss_create(struct selinux_ss **ss)
if (!newss)
return -ENOMEM;
rwlock_init(>policy_rwlock);
+ lockdep_set_class(>policy_rwlock, _ss_class_key);
mutex_init(>status_lock);
+ lockdep_set_class(>status_lock, _status_class_key);
*ss = newss;
return 0;
}
--
2.7.4
Re: [PATCH v2 1/5] selinux:Remove direct references to policydb.
On 01/30/2018 02:46 PM, Stephen Smalley wrote:
> On Fri, 2018-01-26 at 15:32 +0100, peter.enderb...@sony.com wrote:
>> From: Peter Enderborg <peter.enderb...@sony.com>
>>
>> To be able to use rcu locks we seed to address the policydb
>> though a pointer. This preparation removes the export of the
>> policydb and send pointers to it through parameter agruments.
> Just for reference, I have a patch series that does this not only for
> the policydb, sidtab, and class/perm mapping, but for all of the
> SELinux global state, see:
> https://github.com/stephensmalley/selinux-kernel/tree/selinuxns
> and in particular
> https://github.com/stephensmalley/selinux-kernel/commit/c10d90b43cd720c8f8aab51007e805bf7c4f10d2
> https://github.com/stephensmalley/selinux-kernel/commit/ec038a64173d56a331423b6d1564b801f0915afc
> https://github.com/stephensmalley/selinux-kernel/commit/97aa5d7a05e4458bc4562c47d8f7bc4f56fbfefd
>
> Those first three patches should have no effect on SELinux behavior.
> They need to be re-based to latest selinux next branch (some minor
> conflict resolution required) but I was waiting for that to advance to
> something 4.15-rcX based. I could however re-base it now if desired.
I read that as that you want me to rebase the patches on that tree? Seems to
be partly prepared but lot of changes. Is it a moving target?
>> Signed-off-by: Peter Enderborg <peter.enderb...@sony.com>
>> ---
>> security/selinux/ss/mls.c | 69
>> security/selinux/ss/mls.h | 37 +
>> security/selinux/ss/services.c | 90 +++-
>> --
>> security/selinux/ss/services.h | 3 --
>> 4 files changed, 114 insertions(+), 85 deletions(-)
>>
>> diff --git a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c
>> index ad982ce..b1f35d3 100644
>> --- a/security/selinux/ss/mls.c
>> +++ b/security/selinux/ss/mls.c
>> @@ -33,20 +33,20 @@
>> * Return the length in bytes for the MLS fields of the
>> * security context string representation of `context'.
>> */
>> -int mls_compute_context_len(struct context *context)
>> +int mls_compute_context_len(struct policydb *p, struct context
>> *context)
>> {
>> int i, l, len, head, prev;
>> char *nm;
>> struct ebitmap *e;
>> struct ebitmap_node *node;
>>
>> -if (!policydb.mls_enabled)
>> +if (!p->mls_enabled)
>> return 0;
>>
>> len = 1; /* for the beginning ":" */
>> for (l = 0; l < 2; l++) {
>> int index_sens = context->range.level[l].sens;
>> -len += strlen(sym_name(, SYM_LEVELS,
>> index_sens - 1));
>> +len += strlen(sym_name(p, SYM_LEVELS, index_sens -
>> 1));
>>
>> /* categories */
>> head = -2;
>> @@ -56,17 +56,17 @@ int mls_compute_context_len(struct context
>> *context)
>> if (i - prev > 1) {
>> /* one or more negative bits are
>> skipped */
>> if (head != prev) {
>> -nm = sym_name(,
>> SYM_CATS, prev);
>> +nm = sym_name(p, SYM_CATS,
>> prev);
>> len += strlen(nm) + 1;
>> }
>> -nm = sym_name(, SYM_CATS,
>> i);
>> +nm = sym_name(p, SYM_CATS, i);
>> len += strlen(nm) + 1;
>> head = i;
>> }
>> prev = i;
>> }
>> if (prev != head) {
>> -nm = sym_name(, SYM_CATS, prev);
>> +nm = sym_name(p, SYM_CATS, prev);
>> len += strlen(nm) + 1;
>> }
>> if (l == 0) {
>> @@ -86,7 +86,7 @@ int mls_compute_context_len(struct context
>> *context)
>> * the MLS fields of `context' into the string `*scontext'.
>> * Update `*scontext' to point to the end of the MLS fields.
>> */
>> -void mls_sid_to_context(struct context *context,
>> +void mls_sid_to_context(struct policydb *p, struct context *context,
>> char **scontext)
>> {
>> char *scontextp, *nm;
>> @@ -94,7 +94,7 @@ void mls_sid_to_context(struct context *context,
>> struct ebitmap *e;
>> struct ebitmap_node *node;
&g
