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

Message-ID: <063D6719AE5E284EB5DD2968C1650D6D1726AA43@AcuExch.aculab.com>
Date:	Tue, 1 Jul 2014 13:14:32 +0000
From:	David Laight <David.Laight@...LAB.COM>
To:	David Laight <David.Laight@...LAB.COM>,
	Network Development <netdev@...r.kernel.org>,
	"linux-sctp@...r.kernel.org" <linux-sctp@...r.kernel.org>
CC:	"David S. Miller" <davem@...emloft.net>
Subject: RE: [PATCH net-next 2/3] net: sctp: Optimise the way 'sctp_arg_t'
 values are initialised

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' ?

	David




--
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