netdev - Re: [PATCH net-next 2/3] net: sctp: Optimise the way 'sctp_arg

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] [day] [month] [year] [list]

Message-Id: <20140702.184613.1520828698418856140.davem@davemloft.net>
Date:	Wed, 02 Jul 2014 18:46:13 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	David.Laight@...LAB.COM
Cc:	netdev@...r.kernel.org, linux-sctp@...r.kernel.org
Subject: Re: [PATCH net-next 2/3] net: sctp: Optimise the way 'sctp_arg_t'
 values are initialised

From: David Laight <David.Laight@...LAB.COM>
Date: Tue, 1 Jul 2014 13:14:32 +0000

> From: David Laight
>> Even if memset() is inlined (as on x86) using it to zero the union
>> generates a memory word write of zero, followed by a write of the
>> smaller field, and then a read of the word.
>> As well as being a lot of instructions the sequence is unlikely to
>> be optimised by the store-load forward hardware so will be slow.
>> 
>> Instead allocate a field of the union that is the same size as the
>> entire union and write a zero value to it. The compiler will then
>> generate the required value in a register.
>> 
>> Signed-off-by: David Laight <david.laight@...lab.com>
>> ---
>>  include/net/sctp/command.h | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>> 
>> diff --git a/include/net/sctp/command.h b/include/net/sctp/command.h
>> index 020eb95..5fcd1a7 100644
>> --- a/include/net/sctp/command.h
>> +++ b/include/net/sctp/command.h
> ...
>> @@ -212,7 +213,6 @@ typedef struct {
>>   */
>>  static inline int sctp_init_cmd_seq(sctp_cmd_seq_t *seq)
>>  {
>> -	memset(seq, 0, sizeof(sctp_cmd_seq_t));
>>  	return 1;		/* We always succeed.  */
>>  }
> 
> I've realised that this version of this chunk leaves seq->next_free_slot
> and seq->next_cmd uninitialized (they need to be zero).
> The next patch in the series contains different initialisers - which are correct.
> I guess this needs fixing for 'git bisect' ?

Yes, it does.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html