netdev - Re: [PATCH AUTOSEL 4.19 15/68] batman-adv: Expand merged fragment buffer for full packet

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:   Thu, 29 Nov 2018 13:00:22 +0300
From:   Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
To:     Sasha Levin <sashal@...nel.org>, stable@...r.kernel.org,
        linux-kernel@...r.kernel.org
Cc:     Sven Eckelmann <sven@...fation.org>,
        Simon Wunderlich <sw@...onwunderlich.de>,
        netdev@...r.kernel.org
Subject: Re: [PATCH AUTOSEL 4.19 15/68] batman-adv: Expand merged fragment
 buffer for full packet

Hello!

On 11/29/2018 08:55 AM, Sasha Levin wrote:

> From: Sven Eckelmann <sven@...fation.org>
> 
> [ Upstream commit d7d8bbb40a5b1f682ee6589e212934f4c6b8ad60 ]
> 
> The complete size ("total_size") of the fragmented packet is stored in the
> fragment header and in the size of the fragment chain. When the fragments
> are ready for merge, the skbuff's tail of the first fragment is expanded to
> have enough room after the data pointer for at least total_size. This means
> that it gets expanded by total_size - first_skb->len.
> 
> But this is ignoring the fact that after expanding the buffer, the fragment
> header is pulled by from this buffer. Assuming that the tailroom of the

   Pulled by what?

> buffer was already 0, the buffer after the data pointer of the skbuff is
> now only total_size - len(fragment_header) large. When the merge function
> is then processing the remaining fragments, the code to copy the data over
> to the merged skbuff will cause an skb_over_panic when it tries to actually
> put enough data to fill the total_size bytes of the packet.
> 
> The size of the skb_pull must therefore also be taken into account when the
> buffer's tailroom is expanded.
> 
> Fixes: 610bfc6bc99b ("batman-adv: Receive fragmented packets and merge")
> Reported-by: Martin Weinelt <martin@...mstadt.freifunk.net>
> Co-authored-by: Linus Lüssing <linus.luessing@...3.blue>
> Signed-off-by: Sven Eckelmann <sven@...fation.org>
> Signed-off-by: Simon Wunderlich <sw@...onwunderlich.de>
> Signed-off-by: Sasha Levin <sashal@...nel.org>
[...]

MBR, Sergei