netdev - Re: [net-next PATCH 1/5] etherdevice: Add function for handling padding frame to ETH

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, 25 Nov 2014 18:44:46 -0800
From:	Alexander Duyck <alexander.duyck@...il.com>
To:	Florian Fainelli <f.fainelli@...il.com>,
	Alexander Duyck <alexander.h.duyck@...hat.com>,
	netdev@...r.kernel.org
CC:	davem@...emloft.net
Subject: Re: [net-next PATCH 1/5] etherdevice: Add function for handling padding
 frame to ETH_ZLEN

On 11/25/2014 04:56 PM, Florian Fainelli wrote:
> On 25/11/14 14:44, Alexander Duyck wrote:
>> This patch adds a simple function for padding a frame up to the minimum
>> size for for Ethernet.  The motivation behind it is that there are a number
>> of implementations throughout the network device drivers that are all doing
>> the same thing, but each a little bit differently and as a result several
>> implementations contain bugs such as updating the length without updating
>> the tail offset and other similar issues.
>>
>> Signed-off-by: Alexander Duyck <alexander.h.duyck@...hat.com>
>> ---
>>  include/linux/etherdevice.h |   21 +++++++++++++++++++++
>>  1 file changed, 21 insertions(+)
>>
>> diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
>> index 733980f..7e436f3 100644
>> --- a/include/linux/etherdevice.h
>> +++ b/include/linux/etherdevice.h
>> @@ -392,4 +392,25 @@ static inline unsigned long compare_ether_header(const void *a, const void *b)
>>  #endif
>>  }
>>  
>> +/**
>> + * eth_skb_pad - Pad buffer to mininum number of octets for Ethernet frame
>> + * @skb: Buffer to pad
>> + *
>> + * An Ethernet frame should have a minimum size of 60 bytes.  This function
>> + * takes short frames and pads them with zeros up to the 60 byte limit.
> 
> minimum size without FCS
> 
>> + */
>> +static inline int eth_skb_pad(struct sk_buff *skb)
>> +{
>> +	unsigned int size = skb->len;
>> +
>> +	if (unlikely(size < ETH_ZLEN)) {
>> +		size = ETH_ZLEN - size;
>> +		if (skb_pad(skb, size))
>> +			return -ENOMEM;
>> +		__skb_put(skb, size);
>> +	}
> 
> most drivers call skb_padto(skb, ETH_ZLEN), besides the extra
> __skb_put() here, this is not different.

Yes, but the __skb_put is the difference.  Calling skb_padto() only
zeros out the bytes following the tail, but it doesn't add the extra
bytes to the frame nor does it update the tail.  As such if we only call
skb_padto() before handing a short frame off to the network stack the
network stack will still have issues with it being too short.

The difference between the two is that skb_padto() is really meant for
transmit, whereas eth_skb_pad is really meant for receive.

- Alex

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