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: <CAPhsuW5vnVjRS7DWFYPSmq5G-dAFxOrHCAOMkK-iaOoirzMazQ@mail.gmail.com>
References: <1538943795-30895-1-git-send-email-wang6495@umn.edu>
 <CAPhsuW5vnVjRS7DWFYPSmq5G-dAFxOrHCAOMkK-iaOoirzMazQ@mail.gmail.com>
Mime-Version: 1.0
Content-Type: multipart/signed; boundary="==_Exmh_1539033428_2640P";
         micalg=pgp-sha1; protocol="application/pgp-signature"
Content-Transfer-Encoding: 7bit
Date:   Mon, 08 Oct 2018 17:17:08 -0400
Message-ID: <43898.1539033428@turing-police.cc.vt.edu>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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

On Mon, 08 Oct 2018 13:51:09 -0700, Song Liu said:
> On Sun, Oct 7, 2018 at 1:26 PM Wenwen Wang <wang6495@umn.edu> wrote:

> > same value. Given that the btf data is in the user space, a malicious user
> > can race to change the data between the two copies. By doing so, the user
> > can provide malicious data to the kernel and cause undefined behavior.

> These two numbers are copied from same memory location, right? So I
> think this check is not necessary?

Security researchers call this a TOCTOU bug - Time of Check - Time of Use.

What can happen:

1) We fetch the value  (say we get 90) from userspace and stash it in hdr_len.

2) We do some other stuff like check the hdr_len isn't too big, etc..

        meanwhile, on another CPU running another thread of the process...
                3) malicious code stuffs a 117 into that field

4) We fetch the entire header, incliding a now-changed hdr_len  (now 117) and
stick it in btf->hdr->hdr_len.

5) Any code that assumes that hdr_len and btf->hdr->hdr_len are the same value
explodes in interesting ways.


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

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

iQEVAwUBW7vJVI0DS38y7CIcAQL5pwf8Co9z9WwdEhwn1zFByzTwSeNE/fwictdL
EwWkOJb6Q5B8O2QDZrcx2ZSXWEyqDQ8h5gLcyf1q8fenNIfAPM7SkBOUBQEjQMdU
zTwALpsm6IkcoasNe7LpVkVUc2dM4xclyayJOlcahNfDzYiWt1vXsPFN0aGy/Rx0
UQNc09M+6ClxQOcKuQ2leAsvKPghrkfuLOqCBVCaCbCPqrZxI1FDkBg4y3DqOziB
GPBNk0stfa6R5EeWPDsaWXmaCr36e0IWj9mcObUtWtzS+BwR2komgSPJAnoczBaX
0uWV5G/5GfI+tOlk4AcYD7rirsWCFGmDMM/qVLB2NdKbulpabX3XSw==
=m8eg
-----END PGP SIGNATURE-----

--==_Exmh_1539033428_2640P--