netdev - Re: [PATCH v4] net/hsr: Add support for the High-availability Seamless Redundancy protocol (HSRv0)

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, 30 Sep 2013 17:09:35 +0200
From:	Arvid Brodin <arvid.brodin@...n.com>
To:	David Miller <davem@...emloft.net>
CC:	<netdev@...r.kernel.org>, <shemminger@...tta.com>,
	<joe@...ches.com>, <jboticario@...il.com>,
	<balferreira@...glemail.com>, <elias.molina@....es>
Subject: Re: [PATCH v4] net/hsr: Add support for the High-availability Seamless
 Redundancy protocol (HSRv0)

On 2013-09-20 21:10, David Miller wrote:
> From: Arvid Brodin <arvid.brodin@...n.com>
> Date: Thu, 19 Sep 2013 03:11:58 +0200
> 
>> +#if !defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
>> +		/* We need to memmove the whole header to work around
>> +		 * alignment problems caused by the 6-byte HSR tag.
>> +		 */
>> +		memmove(skb_deliver->data - HSR_TAGLEN, skb_deliver->data,
>> +			skb_headlen(skb_deliver));
>> +		skb_deliver->data -= HSR_TAGLEN;
>> +		skb_deliver->tail -= HSR_TAGLEN;
>> +#endif
> 
> You can't do this.
> 
> First of all, you have no idea if subtracting skb->data a given amount
> will underflow the skb buffer start.  You aren't even checking, all
> of the standard skb_*() data adjustment interfaces do.

(Shorter and more to the point than my previous replies:)

I _do_ know: this can't possibly underflow since strip_hsr_tag() a 
few lines above pulled the same amount of data. I will rename 
strip_hsr_tag() to hsr_pull_tag() to make this clearer.


> Secondly, everything after the header is now at the wrong offset from
> the beginning of the packet.

How does this matter? The memmove moves everything back (restores the 
changes made to the packet on the sending side) so that it is at the
"normal" position for an ethernet packet.


-- 
Arvid Brodin | Consultant (Linux)
XDIN AB | Knarrarnäsgatan 7 | SE-164 40 Kista | Sweden | xdin.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