| Message ID | 1479877268-46563-1-git-send-email-vishal.goel@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 11/22/2016 9:01 PM, Vishal Goel wrote: > Add the rcu synchronization mechanism for accessing smk_ipv6_port_list > in smack IPv6 hooks. Access to the port list is vulnerable to a race > condition issue,it does not apply proper synchronization methods while > working on critical section. It is possible that when one thread is > reading the list, at the same time another thread is modifying the > same port list, which can cause the major problems. > > To ensure proper synchronization between two threads, rcu mechanism > has been applied while accessing and modifying the port list. RCU will > also not affect the performance, as there are more accesses than > modification where RCU is most effective synchronization mechanism. > > Signed-off-by: Vishal Goel <vishal.goel@samsung.com> > Signed-off-by: Himanshu Shukla <himanshu.sh@samsung.com> Acked-by: Casey Schaufler <casey@schaufler-ca.com> I have queued this for 4.11 as it's too late for 4.10. > --- > security/smack/smack_lsm.c | 20 +++++++++++++++----- > 1 file changed, 15 insertions(+), 5 deletions(-) > > diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c > index 1cb0602..404919d 100644 > --- a/security/smack/smack_lsm.c > +++ b/security/smack/smack_lsm.c > @@ -52,6 +52,7 @@ > #define SMK_SENDING 2 > > #ifdef SMACK_IPV6_PORT_LABELING > +DEFINE_MUTEX(smack_ipv6_lock); > static LIST_HEAD(smk_ipv6_port_list); > #endif > static struct kmem_cache *smack_inode_cache; > @@ -2604,17 +2605,20 @@ static void smk_ipv6_port_label(struct socket *sock, struct sockaddr *address) > * on the bound socket. Take the changes to the port > * as well. > */ > - list_for_each_entry(spp, &smk_ipv6_port_list, list) { > + rcu_read_lock(); > + list_for_each_entry_rcu(spp, &smk_ipv6_port_list, list) { > if (sk != spp->smk_sock) > continue; > spp->smk_in = ssp->smk_in; > spp->smk_out = ssp->smk_out; > + rcu_read_unlock(); > return; > } > /* > * A NULL address is only used for updating existing > * bound entries. If there isn't one, it's OK. > */ > + rcu_read_unlock(); > return; > } > > @@ -2630,16 +2634,18 @@ static void smk_ipv6_port_label(struct socket *sock, struct sockaddr *address) > * Look for an existing port list entry. > * This is an indication that a port is getting reused. > */ > - list_for_each_entry(spp, &smk_ipv6_port_list, list) { > + rcu_read_lock(); > + list_for_each_entry_rcu(spp, &smk_ipv6_port_list, list) { > if (spp->smk_port != port) > continue; > spp->smk_port = port; > spp->smk_sock = sk; > spp->smk_in = ssp->smk_in; > spp->smk_out = ssp->smk_out; > + rcu_read_unlock(); > return; > } > - > + rcu_read_unlock(); > /* > * A new port entry is required. > */ > @@ -2652,7 +2658,9 @@ static void smk_ipv6_port_label(struct socket *sock, struct sockaddr *address) > spp->smk_in = ssp->smk_in; > spp->smk_out = ssp->smk_out; > > - list_add(&spp->list, &smk_ipv6_port_list); > + mutex_lock(&smack_ipv6_lock); > + list_add_rcu(&spp->list, &smk_ipv6_port_list); > + mutex_unlock(&smack_ipv6_lock); > return; > } > > @@ -2703,7 +2711,8 @@ static int smk_ipv6_port_check(struct sock *sk, struct sockaddr_in6 *address, > return 0; > > port = ntohs(address->sin6_port); > - list_for_each_entry(spp, &smk_ipv6_port_list, list) { > + rcu_read_lock(); > + list_for_each_entry_rcu(spp, &smk_ipv6_port_list, list) { > if (spp->smk_port != port) > continue; > object = spp->smk_in; > @@ -2711,6 +2720,7 @@ static int smk_ipv6_port_check(struct sock *sk, struct sockaddr_in6 *address, > ssp->smk_packet = spp->smk_out; > break; > } > + rcu_read_unlock(); > > return smk_ipv6_check(skp, object, address, act); > } -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 1cb0602..404919d 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -52,6 +52,7 @@ #define SMK_SENDING 2 #ifdef SMACK_IPV6_PORT_LABELING +DEFINE_MUTEX(smack_ipv6_lock); static LIST_HEAD(smk_ipv6_port_list); #endif static struct kmem_cache *smack_inode_cache; @@ -2604,17 +2605,20 @@ static void smk_ipv6_port_label(struct socket *sock, struct sockaddr *address) * on the bound socket. Take the changes to the port * as well. */ - list_for_each_entry(spp, &smk_ipv6_port_list, list) { + rcu_read_lock(); + list_for_each_entry_rcu(spp, &smk_ipv6_port_list, list) { if (sk != spp->smk_sock) continue; spp->smk_in = ssp->smk_in; spp->smk_out = ssp->smk_out; + rcu_read_unlock(); return; } /* * A NULL address is only used for updating existing * bound entries. If there isn't one, it's OK. */ + rcu_read_unlock(); return; } @@ -2630,16 +2634,18 @@ static void smk_ipv6_port_label(struct socket *sock, struct sockaddr *address) * Look for an existing port list entry. * This is an indication that a port is getting reused. */ - list_for_each_entry(spp, &smk_ipv6_port_list, list) { + rcu_read_lock(); + list_for_each_entry_rcu(spp, &smk_ipv6_port_list, list) { if (spp->smk_port != port) continue; spp->smk_port = port; spp->smk_sock = sk; spp->smk_in = ssp->smk_in; spp->smk_out = ssp->smk_out; + rcu_read_unlock(); return; } - + rcu_read_unlock(); /* * A new port entry is required. */ @@ -2652,7 +2658,9 @@ static void smk_ipv6_port_label(struct socket *sock, struct sockaddr *address) spp->smk_in = ssp->smk_in; spp->smk_out = ssp->smk_out; - list_add(&spp->list, &smk_ipv6_port_list); + mutex_lock(&smack_ipv6_lock); + list_add_rcu(&spp->list, &smk_ipv6_port_list); + mutex_unlock(&smack_ipv6_lock); return; } @@ -2703,7 +2711,8 @@ static int smk_ipv6_port_check(struct sock *sk, struct sockaddr_in6 *address, return 0; port = ntohs(address->sin6_port); - list_for_each_entry(spp, &smk_ipv6_port_list, list) { + rcu_read_lock(); + list_for_each_entry_rcu(spp, &smk_ipv6_port_list, list) { if (spp->smk_port != port) continue; object = spp->smk_in; @@ -2711,6 +2720,7 @@ static int smk_ipv6_port_check(struct sock *sk, struct sockaddr_in6 *address, ssp->smk_packet = spp->smk_out; break; } + rcu_read_unlock(); return smk_ipv6_check(skp, object, address, act); }