netdev - Re: [E1000-devel] [PATCH] e100: Fix inconsistency in bad frames handling

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:	Mon, 06 Jun 2011 13:20:57 -0700
From:	Ben Greear <greearb@...delatech.com>
To:	Ben Hutchings <bhutchings@...arflare.com>
CC:	"Brandeburg, Jesse" <jesse.brandeburg@...el.com>,
	Andrea Merello <andrea.merello@...il.com>,
	"e1000-devel@...ts.sourceforge.net" 
	<e1000-devel@...ts.sourceforge.net>, netdev@...r.kernel.org
Subject: Re: [E1000-devel] [PATCH] e100: Fix inconsistency in bad frames handling

On 06/06/2011 01:15 PM, Ben Hutchings wrote:
> On Mon, 2011-06-06 at 10:56 -0700, Ben Greear wrote:
>> On 06/06/2011 10:49 AM, Brandeburg, Jesse wrote:
>>>
>>> <added netdev>, removed other useless lists.
>>>
>>> On Sat, 4 Jun 2011, Andrea Merello wrote:
>>>> In e100 driver it seems that the intention was to accept bad frames in
>>>> promiscuous mode and loopback mode.
>>>> I think this is evident because of the following code in the driver:
>>>>
>>>> if (nic->flags&   promiscuous || nic->loopback) {
>>>> 		config->rx_save_bad_frames = 0x1;	/* 1=save, 0=discard */
>>>> 		config->rx_discard_short_frames = 0x0;	/* 1=discard, 0=save */
>>>> 		config->promiscuous_mode = 0x1;		/* 1=on, 0=off */
>>>> 	}
>>>>
>>>
>>> Hi, thanks for your work on e100.
>>>
>>>> However this intention is not really realized because bad frames are
>>>> discarded later by SW check.
>>>> This patch finally honors the above intention, making the RX code to
>>>> let bad frames to pass when the NIC is in promiscuous or loopback
>>>> mode.
>>>
>>> I think this may be a mistake by the authors of the software developers
>>> manual.  The manual suggests that save bad frames and save short frames
>>> should be enabled in promisc mode, but all of our other drivers *do not*
>>> save bad frames when in promiscuous mode (by design).  This is intentional
>>> because a bad frame is just that, bad, and with no hope of knowing if the
>>> data in it is okay/malicious/other.  I understand your reasoning above,
>>> but realistically the rx_save_bad_frames should NOT be set.  I'd ack a
>>> patch to comment that line out.
>>>
>>>> This helped me a lot to debug an FPGA ethernet core.
>>>> Maybe it can be also useful to someone else..
>>>
>>> I think this patch is just that, debug only. As a developer I understand
>>> why this is useful, but there is no reason any normal user would be able
>>> to benefit from this, so for now, sorry:
>>>
>>> NACK.
>>
>> I think anyone sniffing a funky network would have benefit in
>> receiving all frames.  So, while it shouldn't be enabled by default,
>> it would be nice to have an ethtool command to turn on receiving
>> bad-crc frames, as well as receiving the 4-byte CRC on the end of
>> the packets.
>>
>> It just so happens I have such a patch, in case others agree :)
>
> How would a received skb be flagged as having a CRC error?

We could add an skb flag?  Or just pass it straight up and let
the receiving code deal with it (if we pass up the CRC as well, receiving
code has all info needed).

There are other errors possible as well, I think...maybe short frames, etc,
so it might be nice to have some way to mark what the NIC thinks is wrong
with the pkt.

Thanks,
Ben

-- 
Ben Greear <greearb@...delatech.com>
Candela Technologies Inc  http://www.candelatech.com

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