* [RFC PATCH] selinux: always return a value from the netport/netnode/netif caches
@ 2016-04-13 21:37 Paul Moore
2016-04-13 22:30 ` Daniel Jurgens
2016-04-14 14:09 ` Stephen Smalley
0 siblings, 2 replies; 5+ messages in thread
From: Paul Moore @ 2016-04-13 21:37 UTC (permalink / raw)
To: selinux
From: Paul Moore <paul@paul-moore.com>
Even if we are under memory pressure and can't allocate a new cache
node we can still return the port/node/iface value we looked up from
the policy.
Reported-by: Greg <gkubok@gmail.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
security/selinux/netif.c | 35 +++++++++++++----------------------
security/selinux/netnode.c | 31 +++++++++++++++++--------------
security/selinux/netport.c | 19 ++++++++-----------
3 files changed, 38 insertions(+), 47 deletions(-)
diff --git a/security/selinux/netif.c b/security/selinux/netif.c
index e607b44..5c3bfa4 100644
--- a/security/selinux/netif.c
+++ b/security/selinux/netif.c
@@ -91,18 +91,16 @@ static inline struct sel_netif *sel_netif_find(const struct net *ns,
* zero on success, negative values on failure.
*
*/
-static int sel_netif_insert(struct sel_netif *netif)
+static void sel_netif_insert(struct sel_netif *netif)
{
int idx;
if (sel_netif_total >= SEL_NETIF_HASH_MAX)
- return -ENOSPC;
+ return;
idx = sel_netif_hashfn(netif->nsec.ns, netif->nsec.ifindex);
list_add_rcu(&netif->list, &sel_netif_hash[idx]);
sel_netif_total++;
-
- return 0;
}
/**
@@ -135,7 +133,7 @@ static void sel_netif_destroy(struct sel_netif *netif)
*/
static int sel_netif_sid_slow(struct net *ns, int ifindex, u32 *sid)
{
- int ret;
+ int ret = 0;
struct sel_netif *netif;
struct sel_netif *new = NULL;
struct net_device *dev;
@@ -155,34 +153,27 @@ static int sel_netif_sid_slow(struct net *ns, int ifindex, u32 *sid)
netif = sel_netif_find(ns, ifindex);
if (netif != NULL) {
*sid = netif->nsec.sid;
- ret = 0;
goto out;
}
- new = kzalloc(sizeof(*new), GFP_ATOMIC);
- if (new == NULL) {
- ret = -ENOMEM;
+ ret = security_netif_sid(dev->name, sid);
+ if (ret != 0) {
+ printk(KERN_WARNING
+ "SELinux: failure in sel_netif_sid_slow(),"
+ " unable to determine network interface label (%d)\n",
+ ifindex);
goto out;
}
- ret = security_netif_sid(dev->name, &new->nsec.sid);
- if (ret != 0)
+ new = kzalloc(sizeof(*new), GFP_ATOMIC);
+ if (new == NULL)
goto out;
new->nsec.ns = ns;
new->nsec.ifindex = ifindex;
- ret = sel_netif_insert(new);
- if (ret != 0)
- goto out;
- *sid = new->nsec.sid;
+ new->nsec.sid = *sid;
+ sel_netif_insert(new);
out:
spin_unlock_bh(&sel_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);
- kfree(new);
- }
return ret;
}
diff --git a/security/selinux/netnode.c b/security/selinux/netnode.c
index da923f8..b752bd2 100644
--- a/security/selinux/netnode.c
+++ b/security/selinux/netnode.c
@@ -199,7 +199,7 @@ static void sel_netnode_insert(struct sel_netnode *node)
*/
static int sel_netnode_sid_slow(void *addr, u16 family, u32 *sid)
{
- int ret = -ENOMEM;
+ int ret;
struct sel_netnode *node;
struct sel_netnode *new = NULL;
@@ -210,39 +210,42 @@ static int sel_netnode_sid_slow(void *addr, u16 family, u32 *sid)
spin_unlock_bh(&sel_netnode_lock);
return 0;
}
- new = kzalloc(sizeof(*new), GFP_ATOMIC);
- if (new == NULL)
- goto out;
switch (family) {
case PF_INET:
ret = security_node_sid(PF_INET,
addr, sizeof(struct in_addr), sid);
- new->nsec.addr.ipv4 = *(__be32 *)addr;
break;
case PF_INET6:
ret = security_node_sid(PF_INET6,
addr, sizeof(struct in6_addr), sid);
- new->nsec.addr.ipv6 = *(struct in6_addr *)addr;
break;
default:
BUG();
ret = -EINVAL;
}
- if (ret != 0)
+ if (ret != 0) {
+ printk(KERN_WARNING
+ "SELinux: failure in sel_netnode_sid_slow(),"
+ " unable to determine network node label\n");
goto out;
-
+ }
+ new = kzalloc(sizeof(*new), GFP_ATOMIC);
+ if (new == NULL)
+ goto out;
+ switch (family) {
+ case PF_INET:
+ new->nsec.addr.ipv4 = *(__be32 *)addr;
+ break;
+ case PF_INET6:
+ new->nsec.addr.ipv6 = *(struct in6_addr *)addr;
+ break;
+ }
new->nsec.family = family;
new->nsec.sid = *sid;
sel_netnode_insert(new);
out:
spin_unlock_bh(&sel_netnode_lock);
- if (unlikely(ret)) {
- printk(KERN_WARNING
- "SELinux: failure in sel_netnode_sid_slow(),"
- " unable to determine network node label\n");
- kfree(new);
- }
return ret;
}
diff --git a/security/selinux/netport.c b/security/selinux/netport.c
index 3311cc3..189c293 100644
--- a/security/selinux/netport.c
+++ b/security/selinux/netport.c
@@ -147,7 +147,7 @@ static void sel_netport_insert(struct sel_netport *port)
*/
static int sel_netport_sid_slow(u8 protocol, u16 pnum, u32 *sid)
{
- int ret = -ENOMEM;
+ int ret;
struct sel_netport *port;
struct sel_netport *new = NULL;
@@ -158,13 +158,16 @@ static int sel_netport_sid_slow(u8 protocol, u16 pnum, u32 *sid)
spin_unlock_bh(&sel_netport_lock);
return 0;
}
+ ret = security_port_sid(protocol, pnum, sid);
+ if (ret != 0) {
+ printk(KERN_WARNING
+ "SELinux: failure in sel_netport_sid_slow(),"
+ " unable to determine network port label\n");
+ goto out;
+ }
new = kzalloc(sizeof(*new), GFP_ATOMIC);
if (new == NULL)
goto out;
- ret = security_port_sid(protocol, pnum, sid);
- if (ret != 0)
- goto out;
-
new->psec.port = pnum;
new->psec.protocol = protocol;
new->psec.sid = *sid;
@@ -172,12 +175,6 @@ static int sel_netport_sid_slow(u8 protocol, u16 pnum, u32 *sid)
out:
spin_unlock_bh(&sel_netport_lock);
- if (unlikely(ret)) {
- printk(KERN_WARNING
- "SELinux: failure in sel_netport_sid_slow(),"
- " unable to determine network port label\n");
- kfree(new);
- }
return ret;
}
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [RFC PATCH] selinux: always return a value from the netport/netnode/netif caches
2016-04-13 21:37 [RFC PATCH] selinux: always return a value from the netport/netnode/netif caches Paul Moore
@ 2016-04-13 22:30 ` Daniel Jurgens
2016-04-14 1:20 ` Paul Moore
2016-04-14 14:09 ` Stephen Smalley
1 sibling, 1 reply; 5+ messages in thread
From: Daniel Jurgens @ 2016-04-13 22:30 UTC (permalink / raw)
To: Paul Moore, selinux
On 4/13/2016 4:43 PM, Paul Moore wrote:
> From: Paul Moore <paul@paul-moore.com>
>
> Even if we are under memory pressure and can't allocate a new cache
> node we can still return the port/node/iface value we looked up from
> the policy.
>
> Reported-by: Greg <gkubok@gmail.com>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
> ---
> security/selinux/netif.c | 35 +++++++++++++----------------------
> security/selinux/netnode.c | 31 +++++++++++++++++--------------
> security/selinux/netport.c | 19 ++++++++-----------
> 3 files changed, 38 insertions(+), 47 deletions(-)
>
> diff --git a/security/selinux/netif.c b/security/selinux/netif.c
> index e607b44..5c3bfa4 100644
> --- a/security/selinux/netif.c
> +++ b/security/selinux/netif.c
> @@ -91,18 +91,16 @@ static inline struct sel_netif *sel_netif_find(const struct net *ns,
> * zero on success, negative values on failure.
> *
> */
> -static int sel_netif_insert(struct sel_netif *netif)
> +static void sel_netif_insert(struct sel_netif *netif)
> {
> int idx;
>
> if (sel_netif_total >= SEL_NETIF_HASH_MAX)
> - return -ENOSPC;
> + return;
>
> idx = sel_netif_hashfn(netif->nsec.ns, netif->nsec.ifindex);
> list_add_rcu(&netif->list, &sel_netif_hash[idx]);
> sel_netif_total++;
> -
> - return 0;
> }
>
> /**
> @@ -135,7 +133,7 @@ static void sel_netif_destroy(struct sel_netif *netif)
> */
> static int sel_netif_sid_slow(struct net *ns, int ifindex, u32 *sid)
> {
> - int ret;
> + int ret = 0;
> struct sel_netif *netif;
> struct sel_netif *new = NULL;
> struct net_device *dev;
> @@ -155,34 +153,27 @@ static int sel_netif_sid_slow(struct net *ns, int ifindex, u32 *sid)
> netif = sel_netif_find(ns, ifindex);
I know this is out of context for this patch, but isn't this find
redundant? It was already checked in sel_netif_sid.
> if (netif != NULL) {
> *sid = netif->nsec.sid;
> - ret = 0;
> goto out;
> }
> - new = kzalloc(sizeof(*new), GFP_ATOMIC);
> - if (new == NULL) {
> - ret = -ENOMEM;
> + ret = security_netif_sid(dev->name, sid);
> + if (ret != 0) {
> + printk(KERN_WARNING
> + "SELinux: failure in sel_netif_sid_slow(),"
> + " unable to determine network interface label (%d)\n",
> + ifindex);
> goto out;
> }
> - ret = security_netif_sid(dev->name, &new->nsec.sid);
> - if (ret != 0)
> + new = kzalloc(sizeof(*new), GFP_ATOMIC);
> + if (new == NULL)
> goto out;
> new->nsec.ns = ns;
> new->nsec.ifindex = ifindex;
> - ret = sel_netif_insert(new);
> - if (ret != 0)
> - goto out;
> - *sid = new->nsec.sid;
> + new->nsec.sid = *sid;
> + sel_netif_insert(new);
>
> out:
> spin_unlock_bh(&sel_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);
> - kfree(new);
> - }
> return ret;
> }
>
> diff --git a/security/selinux/netnode.c b/security/selinux/netnode.c
> index da923f8..b752bd2 100644
> --- a/security/selinux/netnode.c
> +++ b/security/selinux/netnode.c
> @@ -199,7 +199,7 @@ static void sel_netnode_insert(struct sel_netnode *node)
> */
> static int sel_netnode_sid_slow(void *addr, u16 family, u32 *sid)
> {
> - int ret = -ENOMEM;
> + int ret;
> struct sel_netnode *node;
> struct sel_netnode *new = NULL;
>
> @@ -210,39 +210,42 @@ static int sel_netnode_sid_slow(void *addr, u16 family, u32 *sid)
> spin_unlock_bh(&sel_netnode_lock);
> return 0;
> }
> - new = kzalloc(sizeof(*new), GFP_ATOMIC);
> - if (new == NULL)
> - goto out;
> switch (family) {
> case PF_INET:
> ret = security_node_sid(PF_INET,
> addr, sizeof(struct in_addr), sid);
> - new->nsec.addr.ipv4 = *(__be32 *)addr;
> break;
> case PF_INET6:
> ret = security_node_sid(PF_INET6,
> addr, sizeof(struct in6_addr), sid);
> - new->nsec.addr.ipv6 = *(struct in6_addr *)addr;
> break;
> default:
> BUG();
> ret = -EINVAL;
> }
> - if (ret != 0)
> + if (ret != 0) {
> + printk(KERN_WARNING
> + "SELinux: failure in sel_netnode_sid_slow(),"
> + " unable to determine network node label\n");
> goto out;
> -
> + }
> + new = kzalloc(sizeof(*new), GFP_ATOMIC);
> + if (new == NULL)
> + goto out;
> + switch (family) {
> + case PF_INET:
> + new->nsec.addr.ipv4 = *(__be32 *)addr;
> + break;
> + case PF_INET6:
> + new->nsec.addr.ipv6 = *(struct in6_addr *)addr;
> + break;
> + }
> new->nsec.family = family;
> new->nsec.sid = *sid;
> sel_netnode_insert(new);
>
> out:
> spin_unlock_bh(&sel_netnode_lock);
> - if (unlikely(ret)) {
> - printk(KERN_WARNING
> - "SELinux: failure in sel_netnode_sid_slow(),"
> - " unable to determine network node label\n");
> - kfree(new);
> - }
> return ret;
> }
>
> diff --git a/security/selinux/netport.c b/security/selinux/netport.c
> index 3311cc3..189c293 100644
> --- a/security/selinux/netport.c
> +++ b/security/selinux/netport.c
> @@ -147,7 +147,7 @@ static void sel_netport_insert(struct sel_netport *port)
> */
> static int sel_netport_sid_slow(u8 protocol, u16 pnum, u32 *sid)
> {
> - int ret = -ENOMEM;
> + int ret;
> struct sel_netport *port;
> struct sel_netport *new = NULL;
>
> @@ -158,13 +158,16 @@ static int sel_netport_sid_slow(u8 protocol, u16 pnum, u32 *sid)
> spin_unlock_bh(&sel_netport_lock);
> return 0;
> }
> + ret = security_port_sid(protocol, pnum, sid);
> + if (ret != 0) {
> + printk(KERN_WARNING
> + "SELinux: failure in sel_netport_sid_slow(),"
> + " unable to determine network port label\n");
> + goto out;
> + }
> new = kzalloc(sizeof(*new), GFP_ATOMIC);
> if (new == NULL)
> goto out;
> - ret = security_port_sid(protocol, pnum, sid);
> - if (ret != 0)
> - goto out;
> -
> new->psec.port = pnum;
> new->psec.protocol = protocol;
> new->psec.sid = *sid;
> @@ -172,12 +175,6 @@ static int sel_netport_sid_slow(u8 protocol, u16 pnum, u32 *sid)
>
> out:
> spin_unlock_bh(&sel_netport_lock);
> - if (unlikely(ret)) {
> - printk(KERN_WARNING
> - "SELinux: failure in sel_netport_sid_slow(),"
> - " unable to determine network port label\n");
> - kfree(new);
> - }
> return ret;
> }
>
>
> _______________________________________________
> Selinux mailing list
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [RFC PATCH] selinux: always return a value from the netport/netnode/netif caches
2016-04-13 22:30 ` Daniel Jurgens
@ 2016-04-14 1:20 ` Paul Moore
0 siblings, 0 replies; 5+ messages in thread
From: Paul Moore @ 2016-04-14 1:20 UTC (permalink / raw)
To: Daniel Jurgens; +Cc: selinux
On Wednesday, April 13, 2016 10:30:26 PM Daniel Jurgens wrote:
> On 4/13/2016 4:43 PM, Paul Moore wrote:
> > From: Paul Moore <paul@paul-moore.com>
> >
> > Even if we are under memory pressure and can't allocate a new cache
> > node we can still return the port/node/iface value we looked up from
> > the policy.
> >
> > Reported-by: Greg <gkubok@gmail.com>
> > Signed-off-by: Paul Moore <paul@paul-moore.com>
> > ---
> >
> > security/selinux/netif.c | 35 +++++++++++++----------------------
> > security/selinux/netnode.c | 31 +++++++++++++++++--------------
> > security/selinux/netport.c | 19 ++++++++-----------
> > 3 files changed, 38 insertions(+), 47 deletions(-)
> >
> > diff --git a/security/selinux/netif.c b/security/selinux/netif.c
> > index e607b44..5c3bfa4 100644
> > --- a/security/selinux/netif.c
> > +++ b/security/selinux/netif.c
> > @@ -91,18 +91,16 @@ static inline struct sel_netif *sel_netif_find(const
> > struct net *ns,>
> > * zero on success, negative values on failure.
> > *
> > */
> >
> > -static int sel_netif_insert(struct sel_netif *netif)
> > +static void sel_netif_insert(struct sel_netif *netif)
> >
> > {
> >
> > int idx;
> >
> > if (sel_netif_total >= SEL_NETIF_HASH_MAX)
> >
> > - return -ENOSPC;
> > + return;
> >
> > idx = sel_netif_hashfn(netif->nsec.ns, netif->nsec.ifindex);
> > list_add_rcu(&netif->list, &sel_netif_hash[idx]);
> > sel_netif_total++;
> >
> > -
> > - return 0;
> >
> > }
> >
> > /**
> >
> > @@ -135,7 +133,7 @@ static void sel_netif_destroy(struct sel_netif *netif)
> >
> > */
> >
> > static int sel_netif_sid_slow(struct net *ns, int ifindex, u32 *sid)
> > {
> >
> > - int ret;
> > + int ret = 0;
> >
> > struct sel_netif *netif;
> > struct sel_netif *new = NULL;
> > struct net_device *dev;
> >
> > @@ -155,34 +153,27 @@ static int sel_netif_sid_slow(struct net *ns, int
> > ifindex, u32 *sid)>
> > netif = sel_netif_find(ns, ifindex);
>
> I know this is out of context for this patch, but isn't this find
> redundant? It was already checked in sel_netif_sid.
The first time we do the cache lookup it is only with the RCU read lock held,
we need to do another lookup once we are holding the spinlock.
--
paul moore
security @ redhat
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH] selinux: always return a value from the netport/netnode/netif caches
2016-04-13 21:37 [RFC PATCH] selinux: always return a value from the netport/netnode/netif caches Paul Moore
2016-04-13 22:30 ` Daniel Jurgens
@ 2016-04-14 14:09 ` Stephen Smalley
2016-04-14 18:49 ` Paul Moore
1 sibling, 1 reply; 5+ messages in thread
From: Stephen Smalley @ 2016-04-14 14:09 UTC (permalink / raw)
To: Paul Moore, selinux
On 04/13/2016 05:37 PM, Paul Moore wrote:
> From: Paul Moore <paul@paul-moore.com>
>
> Even if we are under memory pressure and can't allocate a new cache
> node we can still return the port/node/iface value we looked up from
> the policy.
>
> Reported-by: Greg <gkubok@gmail.com>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
> ---
> security/selinux/netif.c | 35 +++++++++++++----------------------
> security/selinux/netnode.c | 31 +++++++++++++++++--------------
> security/selinux/netport.c | 19 ++++++++-----------
> 3 files changed, 38 insertions(+), 47 deletions(-)
>
> diff --git a/security/selinux/netif.c b/security/selinux/netif.c
> index e607b44..5c3bfa4 100644
> --- a/security/selinux/netif.c
> +++ b/security/selinux/netif.c
> @@ -91,18 +91,16 @@ static inline struct sel_netif *sel_netif_find(const struct net *ns,
> * zero on success, negative values on failure.
> *
> */
> -static int sel_netif_insert(struct sel_netif *netif)
> +static void sel_netif_insert(struct sel_netif *netif)
> {
> int idx;
>
> if (sel_netif_total >= SEL_NETIF_HASH_MAX)
> - return -ENOSPC;
> + return;
This will leak netif (new in the caller). Looks like the other
sel_*_insert() functions handle freeing of the entry if we hit the limit.
>
> idx = sel_netif_hashfn(netif->nsec.ns, netif->nsec.ifindex);
> list_add_rcu(&netif->list, &sel_netif_hash[idx]);
> sel_netif_total++;
> -
> - return 0;
> }
>
> /**
> @@ -135,7 +133,7 @@ static void sel_netif_destroy(struct sel_netif *netif)
> */
> static int sel_netif_sid_slow(struct net *ns, int ifindex, u32 *sid)
> {
> - int ret;
> + int ret = 0;
> struct sel_netif *netif;
> struct sel_netif *new = NULL;
> struct net_device *dev;
> @@ -155,34 +153,27 @@ static int sel_netif_sid_slow(struct net *ns, int ifindex, u32 *sid)
> netif = sel_netif_find(ns, ifindex);
> if (netif != NULL) {
> *sid = netif->nsec.sid;
> - ret = 0;
> goto out;
> }
> - new = kzalloc(sizeof(*new), GFP_ATOMIC);
> - if (new == NULL) {
> - ret = -ENOMEM;
> + ret = security_netif_sid(dev->name, sid);
> + if (ret != 0) {
> + printk(KERN_WARNING
> + "SELinux: failure in sel_netif_sid_slow(),"
> + " unable to determine network interface label (%d)\n",
> + ifindex);
> goto out;
> }
> - ret = security_netif_sid(dev->name, &new->nsec.sid);
> - if (ret != 0)
> + new = kzalloc(sizeof(*new), GFP_ATOMIC);
> + if (new == NULL)
> goto out;
> new->nsec.ns = ns;
> new->nsec.ifindex = ifindex;
> - ret = sel_netif_insert(new);
> - if (ret != 0)
> - goto out;
> - *sid = new->nsec.sid;
> + new->nsec.sid = *sid;
> + sel_netif_insert(new);
>
> out:
> spin_unlock_bh(&sel_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);
> - kfree(new);
> - }
> return ret;
> }
>
> diff --git a/security/selinux/netnode.c b/security/selinux/netnode.c
> index da923f8..b752bd2 100644
> --- a/security/selinux/netnode.c
> +++ b/security/selinux/netnode.c
> @@ -199,7 +199,7 @@ static void sel_netnode_insert(struct sel_netnode *node)
> */
> static int sel_netnode_sid_slow(void *addr, u16 family, u32 *sid)
> {
> - int ret = -ENOMEM;
> + int ret;
> struct sel_netnode *node;
> struct sel_netnode *new = NULL;
>
> @@ -210,39 +210,42 @@ static int sel_netnode_sid_slow(void *addr, u16 family, u32 *sid)
> spin_unlock_bh(&sel_netnode_lock);
> return 0;
> }
> - new = kzalloc(sizeof(*new), GFP_ATOMIC);
> - if (new == NULL)
> - goto out;
> switch (family) {
> case PF_INET:
> ret = security_node_sid(PF_INET,
> addr, sizeof(struct in_addr), sid);
> - new->nsec.addr.ipv4 = *(__be32 *)addr;
> break;
> case PF_INET6:
> ret = security_node_sid(PF_INET6,
> addr, sizeof(struct in6_addr), sid);
> - new->nsec.addr.ipv6 = *(struct in6_addr *)addr;
> break;
> default:
> BUG();
> ret = -EINVAL;
> }
> - if (ret != 0)
> + if (ret != 0) {
> + printk(KERN_WARNING
> + "SELinux: failure in sel_netnode_sid_slow(),"
> + " unable to determine network node label\n");
> goto out;
> -
> + }
> + new = kzalloc(sizeof(*new), GFP_ATOMIC);
> + if (new == NULL)
> + goto out;
> + switch (family) {
> + case PF_INET:
> + new->nsec.addr.ipv4 = *(__be32 *)addr;
> + break;
> + case PF_INET6:
> + new->nsec.addr.ipv6 = *(struct in6_addr *)addr;
> + break;
> + }
> new->nsec.family = family;
> new->nsec.sid = *sid;
> sel_netnode_insert(new);
>
> out:
> spin_unlock_bh(&sel_netnode_lock);
> - if (unlikely(ret)) {
> - printk(KERN_WARNING
> - "SELinux: failure in sel_netnode_sid_slow(),"
> - " unable to determine network node label\n");
> - kfree(new);
> - }
> return ret;
> }
>
> diff --git a/security/selinux/netport.c b/security/selinux/netport.c
> index 3311cc3..189c293 100644
> --- a/security/selinux/netport.c
> +++ b/security/selinux/netport.c
> @@ -147,7 +147,7 @@ static void sel_netport_insert(struct sel_netport *port)
> */
> static int sel_netport_sid_slow(u8 protocol, u16 pnum, u32 *sid)
> {
> - int ret = -ENOMEM;
> + int ret;
> struct sel_netport *port;
> struct sel_netport *new = NULL;
>
> @@ -158,13 +158,16 @@ static int sel_netport_sid_slow(u8 protocol, u16 pnum, u32 *sid)
> spin_unlock_bh(&sel_netport_lock);
> return 0;
> }
> + ret = security_port_sid(protocol, pnum, sid);
> + if (ret != 0) {
> + printk(KERN_WARNING
> + "SELinux: failure in sel_netport_sid_slow(),"
> + " unable to determine network port label\n");
> + goto out;
> + }
> new = kzalloc(sizeof(*new), GFP_ATOMIC);
> if (new == NULL)
> goto out;
> - ret = security_port_sid(protocol, pnum, sid);
> - if (ret != 0)
> - goto out;
> -
> new->psec.port = pnum;
> new->psec.protocol = protocol;
> new->psec.sid = *sid;
> @@ -172,12 +175,6 @@ static int sel_netport_sid_slow(u8 protocol, u16 pnum, u32 *sid)
>
> out:
> spin_unlock_bh(&sel_netport_lock);
> - if (unlikely(ret)) {
> - printk(KERN_WARNING
> - "SELinux: failure in sel_netport_sid_slow(),"
> - " unable to determine network port label\n");
> - kfree(new);
> - }
> return ret;
> }
>
>
> _______________________________________________
> Selinux mailing list
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [RFC PATCH] selinux: always return a value from the netport/netnode/netif caches
2016-04-14 14:09 ` Stephen Smalley
@ 2016-04-14 18:49 ` Paul Moore
0 siblings, 0 replies; 5+ messages in thread
From: Paul Moore @ 2016-04-14 18:49 UTC (permalink / raw)
To: Stephen Smalley; +Cc: selinux
On Thursday, April 14, 2016 10:09:25 AM Stephen Smalley wrote:
> On 04/13/2016 05:37 PM, Paul Moore wrote:
> > From: Paul Moore <paul@paul-moore.com>
> >
> > Even if we are under memory pressure and can't allocate a new cache
> > node we can still return the port/node/iface value we looked up from
> > the policy.
> >
> > Reported-by: Greg <gkubok@gmail.com>
> > Signed-off-by: Paul Moore <paul@paul-moore.com>
> > ---
> >
> > security/selinux/netif.c | 35 +++++++++++++----------------------
> > security/selinux/netnode.c | 31 +++++++++++++++++--------------
> > security/selinux/netport.c | 19 ++++++++-----------
> > 3 files changed, 38 insertions(+), 47 deletions(-)
> >
> > diff --git a/security/selinux/netif.c b/security/selinux/netif.c
> > index e607b44..5c3bfa4 100644
> > --- a/security/selinux/netif.c
> > +++ b/security/selinux/netif.c
> > @@ -91,18 +91,16 @@ static inline struct sel_netif *sel_netif_find(const
> > struct net *ns,>
> > * zero on success, negative values on failure.
> > *
> > */
> >
> > -static int sel_netif_insert(struct sel_netif *netif)
> > +static void sel_netif_insert(struct sel_netif *netif)
> >
> > {
> >
> > int idx;
> >
> > if (sel_netif_total >= SEL_NETIF_HASH_MAX)
> >
> > - return -ENOSPC;
> > + return;
>
> This will leak netif (new in the caller). Looks like the other
> sel_*_insert() functions handle freeing of the entry if we hit the limit.
Yes, good catch.
For a while now I thought we would be better off if we consolidated the
different network object caches into one small cache implementation with
object specific callouts (hash, match, etc.) and cache instances. There is so
much duplicated code between these three and there really is no need for it.
Perhaps I'll play with that this weekend if I get some time.
--
paul moore
security @ redhat
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-04-14 18:49 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-13 21:37 [RFC PATCH] selinux: always return a value from the netport/netnode/netif caches Paul Moore
2016-04-13 22:30 ` Daniel Jurgens
2016-04-14 1:20 ` Paul Moore
2016-04-14 14:09 ` Stephen Smalley
2016-04-14 18:49 ` Paul Moore
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.