netdev - Re: [PATCH] forcedeth: new backoff implementation

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:	Tue, 08 Apr 2008 18:15:32 +0200
From:	Manfred Spraul <manfred@...orfullife.com>
To:	Ayaz Abdulla <aabdulla@...dia.com>
CC:	Andrew Morton <akpm@...ux-foundation.org>,
	Jeff Garzik <jgarzik@...ox.com>, nedev <netdev@...r.kernel.org>
Subject: Re: [PATCH] forcedeth: new backoff implementation

Andrew Morton wrote:
> On Mon, 07 Apr 2008 17:40:28 -0400 Ayaz Abdulla <aabdulla@...dia.com> wrote:
>   
>> +static void nv_gear_backoff_reseed(struct net_device *dev)
>> +{
>> +	u8 __iomem *base = get_hwbase(dev);
>> +	u32 miniseed1, miniseed2, miniseed2_reversed, miniseed3, miniseed3_reversed;
>> +	u32 temp, seedset, combinedSeed;
>> +	int i;
>> +
>> +	/* Setup seed for free running LFSR */
>> +	/* We are going to read the time stamp counter 3 times and swizzle bits around to increase randomness */
>>     
>
> I see this driver rigorously observes the 800-column-xterm convention.
>
>   
>> +	rdtscl(miniseed1);
>>     
>
> err, no.  We don't have sufficient Kconfig dependencies in place to be able
> to do this.  You just broke all non-x86.
>
> Suitable fixes would be
>
> a) add #ifdef CONFIG_X86 all over the place
>
> b) use do_gettimeofday() (might be tricky if called from interrupt)
>
> c) use cpu_clock().
>
>   
d) use get_random_bytes()?
It's irq-safe, used e.g. by the network code for the TCP sequence numbers.
>> +	/* Ensure seeds are not the same */
>> +	if ((combinedSeed & NVREG_BKOFFCTRL_SEED_MASK) ==
>> +	    (combinedSeed & (NVREG_BKOFFCTRL_SEED_MASK << NVREG_BKOFFCTRL_GEAR)))
>> +		combinedSeed = 0x3FF3FF;
>> +
>>     
I don't understand this block: The seed consists of two parts, each 12 bits.
Both parts mustn't be identical. If both parts are identical, then both 
are set to 0x3ff.
But if both are 0x3ff, then they are identical. Is that intentional?

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