netdev - Re: [Patch net] mlx5: check for malformed packets

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]

Message-ID: <3bff30588481e4b4972120590ee74a8836bace4e.camel@mellanox.com>
Date:   Wed, 5 Dec 2018 01:16:06 +0000
From:   Saeed Mahameed <saeedm@...lanox.com>
To:     "saeedm@....mellanox.co.il" <saeedm@....mellanox.co.il>,
        "xiyou.wangcong@...il.com" <xiyou.wangcong@...il.com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Tariq Toukan <tariqt@...lanox.com>
Subject: Re: [Patch net] mlx5: check for malformed packets

On Tue, 2018-12-04 at 12:21 -0800, Cong Wang wrote:
> On Tue, Dec 4, 2018 at 11:33 AM Saeed Mahameed
> <saeedm@....mellanox.co.il> wrote:
> > On Sat, Dec 1, 2018 at 12:38 PM Cong Wang <xiyou.wangcong@...il.com
> > > 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 read it and i understood it, i was just wondering if you are actually
able to reproduce it, and if you have the command line steps to share
with us.

> 
> > 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.

Ok, will be able to start looking at this in a couple of days, sorry
about your time and thanks a lot for the report.

Please understand that RX data path is really sensitive and we are
trying to find the optimal fix of any issue here, sorry for any
inconvenience.

> 
> Thanks.