linux-kernel - Re: [PATCH net] rds: Make sure updates to cp_send

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 20 Jul 2017 13:24:11 +0200
From:   Håkon Bugge <haakon.bugge@...cle.com>
To:     Sowmini Varadhan <sowmini.varadhan@...cle.com>
Cc:     Santosh Shilimkar <santosh.shilimkar@...cle.com>,
        "David S . Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
        OFED mailing list <linux-rdma@...r.kernel.org>,
        rds-devel@....oracle.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net] rds: Make sure updates to cp_send_gen can be observed


> On 20 Jul 2017, at 13:02, Sowmini Varadhan <sowmini.varadhan@...cle.com> wrote:
> 
> On (07/20/17 12:28), H??kon Bugge wrote:
>> cp->cp_send_gen is treated as a normal variable, although it may be
>> used by different threads.
> 
> I'm confused by that assertion. If you look at the comments right
> above the change in your patch, there is a note that 
> acquire_in_xmit/release_in_xmit are the synchronization/serialization 
> points.
> 
> Can you please clarify?

The way the original code works, is that it is allowed for the compiler to keep the value of “cp->cp_send_gen + 1” in a register. The compiler has no requirement to store this value to memory, before leaving the function or calling another one.

Further, said register can be used in the comparison outside the acquire_in_xmit/release_in_xmit, at which point another thread may have changed its value.


Thxs, Håkon

> 
>> --- a/net/rds/send.c
>> +++ b/net/rds/send.c
>> @@ -170,8 +170,8 @@ int rds_send_xmit(struct rds_conn_path *cp)
>> 	 * The acquire_in_xmit() check above ensures that only one
>> 	 * caller can increment c_send_gen at any time.
>> 	 */
>> -	cp->cp_send_gen++;
>> -	send_gen = cp->cp_send_gen;
>> +	send_gen = READ_ONCE(cp->cp_send_gen) + 1;
>> +	WRITE_ONCE(cp->cp_send_gen, send_gen);
>> 
> 
> --Sowmini
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html