On 1/9/2015 5:42 AM, Ahmed S. Darwish wrote:
> Hi Vishal,
>
> On Thu, Jan 08, 2015 at 10:11:52PM +0530, 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.g...@samsung.com>
>> Himanshu Shukla <himanshu...@samsung.com>
> The legality of your patches are blurry. You're sending from
> a personal email, while having Signed-off-by signatures by your
> employer.
>
> You **really** need to add a "From: x...@samsung.com" header on
> the very first line of your emails if this is a sponsored work.
> Kindly check Documentation/SubmittingPatches for further details.
>
> Beside the above:
>
> - Your patches are not applicable to the tree since they're
> white-spaces mangled. You're using Gmail's web interface, which
> is well known at converting tabs to white-spaces. Check
> Documentation/email-clients.txt for further details.
>
> - Please fix you Subject line. Make it something in the form of:
> [PATCH 1/3] smack: Fix xxx
>
> - No need for "[PATCH]" in the commit log body, only in the
> subject line.
>
> - Please make the commit message more comprehensible. Check
> the kernel git log history for good examples. A grammar
> check will also be nice; there are a number of free good
> tools on the web.
>
> - Add "Signed-off-by" headers for each developer. In the patch
> above, you'll need _two_ "Signed-off-by" lines.
>
> - You're sending multiple related patches, but posting each one
> in its own thread. This will make it very very hard for review,
> especially in a very busy list like LKML. Please send related
> patches in an "email thread", with clear sequence numbers.
>
> (e.g., your follow-up patch titled as "In Ref to previous 3
> patches:Fix for synchronization..." is completely bogus.)
>
> Happy kernel coding :-)
>
> Regards,
> Darwish
Further, they still are based on the wrong tree.
These patches need to be based on the smack-next tree:
git://git.gitorious.org/smack-next/kernel.git
branch smack-for-3.20
There has been other work on the IPv6 code for 3.20.
Your patches, even when demangled, do not apply.
>
>> ---
>> 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 majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
