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

Date:	Tue, 08 Apr 2008 15:13:07 -0400
From:	Ayaz Abdulla <aabdulla@...dia.com>
To:	Manfred Spraul <manfred@...orfullife.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

Thanks for the review. I will update the patch and resend it.

Manfred Spraul wrote:
> 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

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
--
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