Re: Fix for synchronization issue in IPV6 implementation in smack module(v3.18)

From: Casey Schaufler
Date: Thu Jan 08 2015 - 19:09:38 EST


On 1/8/2015 8:41 AM, Vishal Goel wrote:
> [PATCH] This patch fixes the synchronization issue in IPv6
> implementation. Previously there was no synchronization mechanism used while
> accessing(adding/reading/deletion) smk_ipv6_port_list. It could be possible
> that when one thread is reading the list, at the same time another thread is
> adding/deleting in the list.So it is possible that reader thread will read
> the inaccurate or incomplete list. So to make sure that reader thread will
> read the accurate list, rcu mechanism has been used while accessing the
> list.RCU allows readers to access a data structure even when it is in the
> process of being updated
>
> Signed-off-by: Vishal Goel <vishal.goel@xxxxxxxxxxx>
> Himanshu Shukla <himanshu.sh@xxxxxxxxxxx>

Your three patches are whitespace mangled.
They also need to be rebased on my current tree:

git://git.gitorious.org/smack-next/kernel.git

using the branch smack-for-3.20. They look OK, but I can't apply them as they are.

Thank you.

> ---
> security/smack/smack_lsm.c | 21 +++++++++++++++------
> 1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index d515ec2..b3427ee 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -52,6 +52,7 @@
> #define SMK_RECEIVING 1
> #define SMK_SENDING 2
>
> +DEFINE_MUTEX(smack_ipv6_lock);
> LIST_HEAD(smk_ipv6_port_list);
>
> #ifdef CONFIG_SECURITY_SMACK_BRINGUP
> @@ -2232,17 +2233,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;
> }
>
> @@ -2258,16 +2262,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.
> */
> @@ -2280,7 +2286,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;
> }
>
> @@ -2335,8 +2343,8 @@ static int smk_ipv6_port_check(struct sock *sk,
> struct sockaddr_in6 *address,
> skp = &smack_known_web;
> goto auditout;
> }
> -
> - 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;
> @@ -2344,6 +2352,7 @@ static int smk_ipv6_port_check(struct sock *sk,
> struct sockaddr_in6 *address,
> ssp->smk_packet = spp->smk_out;
> break;
> }
> + rcu_read_unlock();
>
> auditout:
>
> --
> 1.8.3.2
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/