From mboxrd@z Thu Jan 1 00:00:00 1970 From: Cong Wang Subject: Re: [Patch net] mlx5: check for malformed packets Date: Tue, 4 Dec 2018 12:21:42 -0800 Message-ID: References: <20181201203837.3306-1-xiyou.wangcong@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Cc: Linux Kernel Network Developers , Tariq Toukan , Saeed Mahameed To: saeedm@dev.mellanox.co.il Return-path: Received: from mail-pf1-f175.google.com ([209.85.210.175]:42841 "EHLO mail-pf1-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725797AbeLDUVy (ORCPT ); Tue, 4 Dec 2018 15:21:54 -0500 Received: by mail-pf1-f175.google.com with SMTP id 64so8766554pfr.9 for ; Tue, 04 Dec 2018 12:21:53 -0800 (PST) In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: On Tue, Dec 4, 2018 at 11:33 AM Saeed Mahameed wrote: > > On Sat, Dec 1, 2018 at 12:38 PM Cong Wang wrote: > > > > is_last_ethertype_ip() is used to check IP/IPv6 protocol before > > parsing IP/IPv6 headers. > > > > But __vlan_get_protocol() is only bound to skb->len, a malicious > > packet could exhaust all skb->len by inserting sufficient ETH_P_8021AD > > headers, and it may not even contain an IP/IPv6 header at all, so we > > have to check if we are still safe to continue to parse IP/IPv6 header. > > If not, treat it as non-IP packet. > > > > This should not cause any crash as we stil have tail room in skb, > > but we can't just rely on it either. > > Hi Cong, is this reproducible or just a theory ? which part of the > code you think will cause the invalid access or crash ? Since you don't even read into my changelog, here it is: "This should not cause any crash as we stil have tail room in skb, but we can't just rely on it either." As I already explained to you in a private email, when we reference whatever field in struct iphdr, we have to make sure the offset of that field is within skb->len. > do you have steps to reproduce this? > Again, you really have to read the changelog I wrote: "a malicious packet could exhaust all skb->len by inserting sufficient ETH_P_8021AD headers, and it may not even contain an IP/IPv6 header at all, " > I would like to investigate this myself, it will take a couple of days > if that's ok with you .. Sure, take your time. I am sending the patch only for showing the problem, NOT to merge. Let's discard it anyway. I am wasting my time. Thanks.