Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH AUTOSEL 4.19 15/68] batman-adv: Expand merged fragment
 buffer for full packet
From:   Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
To:     Sasha Levin <sashal@kernel.org>, stable@vger.kernel.org,
        linux-kernel@vger.kernel.org
Cc:     Sven Eckelmann <sven@narfation.org>,
        Simon Wunderlich <sw@simonwunderlich.de>,
        netdev@vger.kernel.org
References: <20181129055559.159228-1-sashal@kernel.org>
 <20181129055559.159228-15-sashal@kernel.org>
 <cad9ec40-5125-9bef-0e9e-5c70783a5cb7@cogentembedded.com>
Organization: Cogent Embedded
Message-ID: <3da190f1-254a-28d8-3219-1a129c5b8fda@cogentembedded.com>
Date:   Thu, 29 Nov 2018 13:04:42 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.2.1
MIME-Version: 1.0
In-Reply-To: <cad9ec40-5125-9bef-0e9e-5c70783a5cb7@cogentembedded.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-MW
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 11/29/2018 01:00 PM, Sergei Shtylyov wrote:

>> From: Sven Eckelmann <sven@narfation.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?

   Oops, this was a -stable patch! Nevermind then. :-)

>> 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@darmstadt.freifunk.net>
>> Co-authored-by: Linus Lüssing <linus.luessing@c0d3.blue>
>> Signed-off-by: Sven Eckelmann <sven@narfation.org>
>> Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
>> Signed-off-by: Sasha Levin <sashal@kernel.org>
> [...]
 
MBR, Sergei