From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sergei Shtylyov Subject: Re: [PATCH AUTOSEL 4.19 15/68] batman-adv: Expand merged fragment buffer for full packet Date: Thu, 29 Nov 2018 13:04:42 +0300 Message-ID: <3da190f1-254a-28d8-3219-1a129c5b8fda@cogentembedded.com> References: <20181129055559.159228-1-sashal@kernel.org> <20181129055559.159228-15-sashal@kernel.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: Sven Eckelmann , Simon Wunderlich , netdev@vger.kernel.org To: Sasha Levin , stable@vger.kernel.org, linux-kernel@vger.kernel.org Return-path: In-Reply-To: Content-Language: en-MW Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On 11/29/2018 01:00 PM, Sergei Shtylyov wrote: >> From: Sven Eckelmann >> >> [ 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 >> Co-authored-by: Linus Lüssing >> Signed-off-by: Sven Eckelmann >> Signed-off-by: Simon Wunderlich >> Signed-off-by: Sasha Levin > [...] MBR, Sergei