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;
From:   valdis.kletnieks@vt.edu
To:     Song Liu <liu.song.a23@gmail.com>
Cc:     wang6495@umn.edu, kjlu@umn.edu,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Networking <netdev@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] bpf: btf: Fix a missing check bug
In-Reply-To: <CAPhsuW5nPGMOEMKa1UFwgquoUeYiUV8i8nVgWfM6McH2Ri1esg@mail.gmail.com>
References: <1538943795-30895-1-git-send-email-wang6495@umn.edu> <CAPhsuW5vnVjRS7DWFYPSmq5G-dAFxOrHCAOMkK-iaOoirzMazQ@mail.gmail.com> <43898.1539033428@turing-police.cc.vt.edu>
 <CAPhsuW5nPGMOEMKa1UFwgquoUeYiUV8i8nVgWfM6McH2Ri1esg@mail.gmail.com>
Mime-Version: 1.0
Content-Type: multipart/signed; boundary="==_Exmh_1539047240_7716P";
         micalg=pgp-sha1; protocol="application/pgp-signature"
Content-Transfer-Encoding: 7bit
Date:   Mon, 08 Oct 2018 21:07:20 -0400
Message-ID: <9337.1539047240@turing-police.cc.vt.edu>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

--==_Exmh_1539047240_7716P
Content-Type: text/plain; charset=us-ascii

On Mon, 08 Oct 2018 17:44:46 -0700, Song Liu said:

> I think I get the security concept here. However, hdr_len here is only used to
> copy the whole header into kernel space, and it is not used in other
> logic at all.
> I cannot image any security flaw with either hdr_len > btf->hdr->hdr_len case or
> hdr_len < btf->hdr->hdr_len. Could you please provide more insights on what
> would break by malicious user space?

Say the biggest allowed value for hdr_len is 128.  We check the value, the user has 98.
They then stuff 16,383 into there.

Now here's the problem - hdr_len is a local variable, and evaporates when the function
returns.  From here on out, anybody who cares about the header length will use the
value in btf->hdr_len....

(And yes, somebody *does* care about the length, otherwise we wouldn't need a field
saying what the length was....)

Now think how many ways that can go pear-shaped.  You copied in 98 bytes, but outside
the function, they think that header is almost 4 pages long.  Does that ever get used as
a length for kmemcpy()?  Or a limit for a 'for (i=start; i< (start+hdr->hdr_len); i++)' that
walks across a variable length header?

Can you cook up a way to have a good chance to oops the kernel when it walks off the
page you allocated the 98 bytes on?  Can you use it to export chunks of memory out to
userspace?  Lots and lots of ways for this to kersplat a kernel...;

--==_Exmh_1539047240_7716P
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Comment: Exmh version 2.8.0 04/21/2017

iQEVAwUBW7v/SI0DS38y7CIcAQKEFgf9E+sMPZmhYDvR8pRqDx1ieuHDbfnaMBNm
PDfnp/S5xMWASD9bEEGu5xtSYC9aR1GjTFFtyW/ytz1wIpXHyf7XZgOf4HqJfrkK
DqqTOxkzkuTcyNafTbyrv3Mpzu/VhnHnjnRv45BYWCOXGJzXALgjeyvywJcJuzkH
7Vd0n0GGc+aHxBhAbTYPx1es+lrD9MaxJzMwbPp0u6zMAIFr1GMtwihu1GfDjEM+
1zLllX8a/tIEaCpTbQuYli8gWVbMfWELartMBm8taFfppIj+/op4UMH79FZY96gy
8nTQZgvIlIWbNrAyydG6P9lPFTBT/d7n9c7TQeCWkwoPRvBU7pB1GQ==
=h6iu
-----END PGP SIGNATURE-----

--==_Exmh_1539047240_7716P--