From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3719BC3DA41 for ; Wed, 10 Jul 2024 20:53:44 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id BF86C6B00A0; Wed, 10 Jul 2024 16:53:43 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id BA8ED6B00A1; Wed, 10 Jul 2024 16:53:43 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A705F6B00A3; Wed, 10 Jul 2024 16:53:43 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 87AC96B00A0 for ; Wed, 10 Jul 2024 16:53:43 -0400 (EDT) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 21DDEC0291 for ; Wed, 10 Jul 2024 20:53:43 +0000 (UTC) X-FDA: 82325044326.18.AAF41C4 Received: from mail-ej1-f51.google.com (mail-ej1-f51.google.com [209.85.218.51]) by imf03.hostedemail.com (Postfix) with ESMTP id 4CA9C20005 for ; Wed, 10 Jul 2024 20:53:41 +0000 (UTC) Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=bDaW4sA7; spf=pass (imf03.hostedemail.com: domain of andrii.nakryiko@gmail.com designates 209.85.218.51 as permitted sender) smtp.mailfrom=andrii.nakryiko@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1720644806; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=DkK9U166sGyL2mgKNrTUkPuaSLctLnuD4jDcoEWUIFY=; b=S41yXusTKC4dpL4pP+Vp/CzbH2yBofFe/4f/ub6CoLMob4WjhXz+JBhu7yd7K7/tXLPz7Y rv7IEdnG1yzxyC1OWKy5CXxAlbBH0GAEmnESvMw5UUzy09QPrh9tgzNr+VYD0C9C+j+gk+ rCv5Wlsk22yedJ2lkvg8euyJr1I9E68= ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=bDaW4sA7; spf=pass (imf03.hostedemail.com: domain of andrii.nakryiko@gmail.com designates 209.85.218.51 as permitted sender) smtp.mailfrom=andrii.nakryiko@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1720644806; a=rsa-sha256; cv=none; b=UX9/2xZGDcNkG1GNOM36LHrliw/AQ1jAQR6b0eRrWPnsJ95tP+Z81Vho6dLKpRAf/8gMBQ JyuGD/h2Q7xvIiFqxMQQoKX60W0q+v45DCMDZVMS7T2nydoG1nY20cZYjOvOoHBHleBZ3u GloIAlDZM3jAtHAbBE0NfcB6n7isONY= Received: by mail-ej1-f51.google.com with SMTP id a640c23a62f3a-a77bf336171so39861766b.1 for ; Wed, 10 Jul 2024 13:53:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1720644820; x=1721249620; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=DkK9U166sGyL2mgKNrTUkPuaSLctLnuD4jDcoEWUIFY=; b=bDaW4sA7tZ2kmyuNga6+E2R17ef5ydY8reOhQk4HsEN13/1Hf5QZsdb/j9qrn6GM8P mh98zcqWNcYnUycbOxAgRT1EjalJswfmh5umaGCMs/MdfwN+nGqMH3qw+pb6q/HTS4U4 lNWSDmJRtkrx9ULQAeE1sifK7YBiB/TU1O+hmBqto+BotTHCdCWA2aSRsmAB9AN1Gl7O NUTJ5jU06xob90CvYZLzrEX/pFWvU8ruJ/tqZQe566rlfEu6h6ihZc3AMzBtOMk50Qwu scyXHmhs7CXGiqvxahA0nNKEXF0l5G5Y1pYDGCY5C9wUpHhUZctikgKdQRzjt5Dr0d5o vONw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1720644820; x=1721249620; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=DkK9U166sGyL2mgKNrTUkPuaSLctLnuD4jDcoEWUIFY=; b=FmTeF/4FSwBQUw/bui8H1WjGJRn+vJggtn50V87ZadwSSojhjjlBfwkI9EQoqkIbdk BFm9ZC68mVlEqkAnM3L8IG14FKrmgBDX6xJE/ciTRM+XopTj74V3+hoV2uY49Be84JZt TrBRtBbSzhesf4ChUQ7ffLAjt/mp2e+WBxoit2Y+PS/NgCeG2DAwbsP/RN+ur+kN//id CSNVdcmvTl/vM4Bzy+TCdNvNtBT3Iw8u4QIAzrlByhaqAXU/7tH9DWpxlnWzC4Cfi0q9 g747bg0xLVWty2PFh/N0mq94V/ZLDPkR7LiaK0pVjjmzGisxQfK2LpssYxhOBvJoe46g BeFA== X-Forwarded-Encrypted: i=1; AJvYcCVNddmLd/dr2pzw1h4X3SS7g9usdr/71rDb/Os5DtaGdOaTioaY4WW59LF0+iSO1BQEzVihyjDBzbhRknjXHk580Tk= X-Gm-Message-State: AOJu0YzZdHSi6gv8C8SDB1x1XM/NRoOf5syW6mo/kK8Es8cPJyxJ7Lyc uplTC2YK5g8++p/yVkN8vMILlehjhNxSQo7um8oZnTUln5u355/KnZzMlCI3SxxHT90WlTD/s90 Fv1M141V+xgxPJHQprRXNjOwP4Ew= X-Google-Smtp-Source: AGHT+IGsaPHTAeph1lS6C7ms6qfP7x13Ux3MujW8K/UUrVmwsG+Vy5csF32yPz82/fB8aAkInrdNbV8G7zu++GxliQU= X-Received: by 2002:a17:906:480c:b0:a77:ce4c:8c9c with SMTP id a640c23a62f3a-a798a291a21mr55853466b.8.1720644819430; Wed, 10 Jul 2024 13:53:39 -0700 (PDT) MIME-Version: 1.0 References: <20240709204245.3847811-1-andrii@kernel.org> <20240709204245.3847811-8-andrii@kernel.org> In-Reply-To: From: Andrii Nakryiko Date: Wed, 10 Jul 2024 13:53:23 -0700 Message-ID: Subject: Re: [PATCH bpf-next 07/10] lib/buildid: harden build ID parsing logic some more To: Andi Kleen Cc: Andrii Nakryiko , bpf@vger.kernel.org, linux-mm@kvack.org, akpm@linux-foundation.org, adobriyan@gmail.com, shakeel.butt@linux.dev, hannes@cmpxchg.org, osandov@osandov.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: 4CA9C20005 X-Stat-Signature: 9yd9i4yw69db94x7m7pzatyxpxftiipt X-HE-Tag: 1720644821-727723 X-HE-Meta: U2FsdGVkX19z4RLkKh3BfqtSM9kZ/T6m2m1581RXJmjAXPSaWFtZl1ksZ4F4uuV9lZS/Yf78xpeWRejpBouMDMxhwH8RP4ClPAx3145u52cDzIVNe4Td4vs3XJnx+kqgv/Oy3otPkeplPjuKOmTpk7fUmgquX0RDNGRP5GUGyWsft2NuUSLDHOR42U95T5jz/5Q/KUOpCmghBKF1zi1OTVDSPJzw71bZPK+l2Z2kXDxUnkbvsa4+zeTacMrwB8C6Z6RFQoWqyRW4dyNYkh2wLfkanGfoEzK5vw6PXjUe5d8XUKexGKfYzwcO8xB87P+bs1Pw47oDOQApnC9e0hGm3SMuM2HuctDXHxLaS/7u7LdiR4fQOAs5u4NKfXUtHyLhg6tm23mDwnN+2VH84QZqLRErM6J02LQuG9aUG6RgPRtRh0aZUOZbvFSgk95V4eNrFe1UFLWb0pr09xYQA2eDiwFMWYjHmXMBWyDM+txJtd4PysSLb6XOLb3TX3d8vlEQjemXkduRt4XgGic9E9scgBF8rwoe9jcDj3XM0ewtrwuciG+JrpLUU2HFcqK2Lb51LCTTVThnB1+G1rF50q2yyeE3XqkCXJKeyAlcymJHsfeesPBmp1vpRv9HFJhPXmX9uFSWFB1Yw3ETrF+vLHn7LpQrY+VsIeKxMjj+2l9SyzfQkrXL35YbHST7P1eWOnTUHkIxu5S7tudAysqj/abl/cmm7Ymj9NT3Cvfp5TahrWfbUC+Mw0r4V4cnBnmCfh4cQHrigjcryLPaAf9PmsWOvqwK3DVn9ksWGQ5c8nhqUvoyAdJKIAP3nxYCa4wLyW5s5VIdfFYVUSRnvLM0WSeggx0tlCKKwGZSFcbcLXPY4CmNNk1mgBEPmrsivE9lko7TN8VDvTM9Lys+hJQ8k/C91bRmengK14OQpe9fuXhwQr3RKibZP43XU9DSw0iwgSg4JZH3zp/KCQ62j1n+/vf GWNtSHL1 NU5DKverObXjOI8eBOlyzdnCLrRNzfyl8gq4oaMTrN38/aukhBzXjk8UMZp1ViAaH4s9voAhSMXfVi2eMnWC3Lm7JAln0heMbnsxfk3RNGnIFOnId9svApqgen5R2pEk39SYA4mR7N0ZWkv5q3QnpIdbhQEZfL+F8Sbo1Vyjkatq4aiO1mBDp7X24IuUBW/KhDPpKnuZBEgMwEP/5fZ7etv3kLzygd5dRPmZx2RCa0AuouA5JZIug6gYbQZ0qhBmTw7v4G2EYMpi2X+zPUKBZdB7pJggdvzaqo0eaImUp3aghV37Ch/LCUBeWAWgob/2cpKjjH3Ce1h7RibmdipKrzYNdkdrV7GAWHWiw X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Wed, Jul 10, 2024 at 1:45=E2=80=AFPM Andi Kleen wro= te: > > On Tue, Jul 09, 2024 at 01:42:42PM -0700, Andrii Nakryiko wrote: > > Harden build ID parsing logic some more, adding explicit READ_ONCE() > > when fetching values that we then use to check correctness and various > > note iteration invariants. > > Just sprinkling READ_ONCE all over doesn't necessarily fix the code. > It is only needed for values that affect a loop or reference. Agreed, besides `READ_ONCE(nhdr->n_type) =3D=3D BUILD_ID` and `READ_ONCE(phdr->p_type) =3D=3D PT_NOTE`, which I added mostly just for consistency, the rest should be indeed read once and then checked, no? Do you see any other unnecessary READ_ONCE()s in this patch? > > You have to fix stuff like this > > static inline int parse_build_id(const void *page_addr, > unsigned char *build_id, > __u32 *size, > const void *note_start, > Elf32_Word note_size) > { > /* check for overflow */ > if (note_start < page_addr || note_start + note_size < note_start= ) > ^^^^^^^^^^^^^^^^^^^^^^ this has been switched to u64-based offsets in patch #1, did you take a look at it? > return -EINVAL; > > > which is C undefined (at least without -fwrapv-pointer) and can easily > be miscompiled if it isn't already. > > I suspect the code will need more work, especially since you're > unwilling to consider any defense in depth measures. > Can you be a bit more specific about the remaining issues? I'm happy to fix whatever can and should be fixed (after the changes I already did in this patch set). If by "defense in depth" you mean allowing this functionality only for executable VMAs, then yes, I refuse to do that, as I already explained. > -Andi >