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 E4F88E6F07D for ; Fri, 1 Nov 2024 18:13:12 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 143316B0085; Fri, 1 Nov 2024 14:13:12 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 0F37C6B0088; Fri, 1 Nov 2024 14:13:12 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EFCEA6B0089; Fri, 1 Nov 2024 14:13:11 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id D270D6B0085 for ; Fri, 1 Nov 2024 14:13:11 -0400 (EDT) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 804BEA0CA1 for ; Fri, 1 Nov 2024 18:13:11 +0000 (UTC) X-FDA: 82738321806.27.4271EAB Received: from mail-pf1-f169.google.com (mail-pf1-f169.google.com [209.85.210.169]) by imf27.hostedemail.com (Postfix) with ESMTP id AACC64000E for ; Fri, 1 Nov 2024 18:12:40 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=G7E6bMWK; spf=pass (imf27.hostedemail.com: domain of andrii.nakryiko@gmail.com designates 209.85.210.169 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=1730484657; 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=PiW9SEzE14uWJXxy3FOB2Ud96GmiPqBall1rSE+SePk=; b=RBi32CqUMX0yQxF1pvmE6fTgGiZE+huhvKGcz4wQGpXdN+k3Mv7gkaJ86hnKOhNqxIDqts gTHvb9cUNp4kGCWTiM/WjeIJzTWOU7DlNYmyCUtPWtewqicVJaaVlP9cYf8yXH+V7ceuvX J2Qyf6Ppi+pvGiRuYIg3z1gpTC584gs= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1730484657; a=rsa-sha256; cv=none; b=DShZ9ghUCQOxrmQ0PNxOdyoRNxB4+mSGn/kymLxtWZ0s3Y9Is8RM3SDlzRNCa5n+exhMwv Noiuv6HcVVAuhi8gbSvkFKr/czhw7uk9o8OJdDSfxYZLOgAVvN9I7ydGydc0WNM52GTcux Orw8dR3lqZtAAn3jg/f1FlccukkYq4A= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=G7E6bMWK; spf=pass (imf27.hostedemail.com: domain of andrii.nakryiko@gmail.com designates 209.85.210.169 as permitted sender) smtp.mailfrom=andrii.nakryiko@gmail.com; dmarc=pass (policy=none) header.from=gmail.com Received: by mail-pf1-f169.google.com with SMTP id d2e1a72fcca58-720cb6ac25aso1091300b3a.3 for ; Fri, 01 Nov 2024 11:13:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1730484788; x=1731089588; 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=PiW9SEzE14uWJXxy3FOB2Ud96GmiPqBall1rSE+SePk=; b=G7E6bMWK+7BAf4bPJ5yT3l6Epk2AdslFQMKbqY/LQYSA1guW2/C0rLxsFY+bFaRPKx dyPVGPc8V0ZQdsT9QVKEFh49bqXDi2mGqxyW8vmG0ViXctZCtXkBc3KdZ4tTakYJANYI zunq2vdNev5RKV+n0Myc+5YAEWlA9ofMt3Ob8rFtKM2URwJeNgRUXWz/pdj0VU2l24wo LnZsfPnZnTK0w45CyQ3mLV289mzYhVSlRazaxYWPS/6wQ8p7NF/pT18Lnfao868Mn38v PC+hvzuXz6H0e6jSFYBe0YlPDxy21ZX4EX+H6DJAHrIbt9IbJWM5QlC6Sz2RsGjGNeev SRQw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730484788; x=1731089588; 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=PiW9SEzE14uWJXxy3FOB2Ud96GmiPqBall1rSE+SePk=; b=IAOtpBdKDSQ9TZK5aK8drb35Q0G8Jp76mlyIKEUkneQNwU+d5aQPDb5k+c6v3ue1hw 4+guQk45w4mKXmkmaCBTAzwdb6+P4Dta20eeNRmAry47MttWgXgTosoRkmRM8i/AOjLH uoR6+moE4V7vV1Yy+5P5CHUOeKyXgh9DoaT4K3UQ0KGYscn9zTP6+hmTJ6zXTBbIfGTI M3ZSarTi/8LxE4+nT9Ysxznf0i4uqdoR04TE64IZLeB2NStuV1H4asDRIQlg4r63jifG mVJBtDXRyVG6iWPY7jfMXZXMmwbs+aorYBbGy8eYqviuA9fMkSNTFwz3WPG1kHBggPKx iOAw== X-Forwarded-Encrypted: i=1; AJvYcCWhDNS/zj+RwatMFcVCAP4nJ6iXYCBm+8ahH6vyJCnRzd8LjaYCqNQ5tAFdEQ+vmYGKJ2j6Gr1KZg==@kvack.org X-Gm-Message-State: AOJu0YwMPvnuaXntcvN8I3NQDeqXMb7ihPOInDlONNpiDQ/jLrfOkvxQ 0P2M4bmOp1b7tV6MOKBymImhFY+Y+5Fi44VOKsNvmHf/FfWTb7XwOlLywVyPS9mQOtgD4Ovxh1N 1vQOx6p+AA2aNqZTdyAKcUl/9y8c= X-Google-Smtp-Source: AGHT+IEk5xx3/JFmK3SmabSdACLb2Y3za3f+3kE6RcmNc7n4e0XhM9sTxifeXbsjGY0xW5T523PyYTIoJXLl8NRIFS0= X-Received: by 2002:a05:6a21:78c:b0:1d9:261c:5942 with SMTP id adf61e73a8af0-1d9a8403ce0mr30567357637.28.1730484788340; Fri, 01 Nov 2024 11:13:08 -0700 (PDT) MIME-Version: 1.0 References: <20240829174232.3133883-1-andrii@kernel.org> <20240829174232.3133883-2-andrii@kernel.org> In-Reply-To: From: Andrii Nakryiko Date: Fri, 1 Nov 2024 11:12:55 -0700 Message-ID: Subject: Re: [PATCH v7 bpf-next 01/10] lib/buildid: harden build ID parsing logic To: Jiri Olsa 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, ak@linux.intel.com, osandov@osandov.com, song@kernel.org, jannh@google.com, linux-fsdevel@vger.kernel.org, willy@infradead.org, stable@vger.kernel.org, Eduard Zingerman Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: AACC64000E X-Stat-Signature: ho4iike8w4bxwd8363ae5kxp1uc9ked7 X-HE-Tag: 1730484760-92389 X-HE-Meta: U2FsdGVkX1/6xABWqXAJU3+ueAkI3oFUsY8CbkWN/zwfvGpDHjOJ4kLOfRncqqUSe2W94PmdYS7DLYTfAg6bSxYbZ3jX2tgmHmbFcMvfqZQ9aK/0rL4NEPWsEas+WQn6ZuCQSA/mtli8DjjsOHAUV0ftYapbblP/d17lbe85rXW3lSHNPQGAKhCn3qL430+Gc+wmPGjRO5AL6izNss+ZugxxYrwxZd1zW8I4kguBToy8rcIkl9tYs0+FqjsPBICLl3ybqyEcK5ks9ORtuTqFKzBm5tWKpJhUNPKXmOKjs3MHz5dFqXWUbpBq9RszyuDOiCC9AV7pmWRr/aXbFW/bdQtx9LGcL5VTR6yaR55haGVQi1YD5iofgtFJ0MBDJxMBN5nwG+0WbSngTwO4OlYTHmtGaGg5Qnn4o9UFXh6OeqBC6gPBCMLD/0Q5nYjPPwT0/oJ+9Wv2Y6pRDsYJJsPkwyNJeCgW+RhWKpqEbSkgWwygpc8pK1MJznrhSDW2eh/ccIyAADO4b1EebDZAm+SE9bJs9SrGaTGr6d9W3ofqKswyUP8ANANdCGZjra0dZfSyTrVLly6JgsHClX+pW+lqt4nwzDbAlj+W+c8RiIAbNIW/nAvILVEHhjqkN7qaq/HBYsL57g/k0zDm3WYwCWxHT39x7QFcNCJ99rkTW8MJV0XY2FMIi2rArqjphNJEXySMniX2QJeH/Vuy88/XBccHCNP7AEJHcEmATfRlR5+xxLgOnw0EiNvtKVC0XvrLeczS0nvPvWEvv8RIvJykJ010dL3/SbvTWZtqqy7jcqt7dz2WGRXxEsi0/ONPqquX2qxy5S9J+RmOac/ydmroFS0Q7tWDpKixSu8aan85PP4eqWNch/O95H2YVyL/TOlvK5l0Z7X1d85l+xBjMPCLCNLrwOFZnDNXzR6ikDu2vvpaLj5uvumRvzrLOQrIXMUFm/0FG4Ha9aP1Oou3Msruh/h kiDi4Yf9 vcqKXzoIkzF0Ue/dXoQOFAsGBf+Cf1zEUIXEafTBY0HCboE7x+Ex8Kd4jfgUi2iAGyWpVcYM9UMM2wzjAeaYs0zM9edrTwq1H8AsNZQdjTmtsujnrXCBIAu4fCkylKLyodU/ehwBEYgXKMVt2ItjAPfYCIY7SjWur079RTk9CxXTvzXm+yuLjZKu5KV9w6DqmAHpctqK7hrOWau2AohwmSuqIFxF+u/Or7C0kEToRJmEG9a7njcI3ak8X3A6kRrVR0yOCTz8KnApL72Zn9tbJll5Ow4Y5UNorcOca/c6nHX1vpFEIK13dvR4LjhCtE7D3IH3xHzZQiFdRrOUmdxmGJ4vQoZ4ttbMjGzuXLCl48uK5BhnoV5T5+1uh7jbOzyDP9YtMJFH5H66H0oG2ALl4EC5PkUIRg5WEwDZxO7RAQ2vLtPYmvBewLacCwdSq5Lh6pcY+ 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 Fri, Nov 1, 2024 at 6:54=E2=80=AFAM Jiri Olsa wrote= : > > On Thu, Aug 29, 2024 at 10:42:23AM -0700, Andrii Nakryiko wrote: > > Harden build ID parsing logic, adding explicit READ_ONCE() where it's > > important to have a consistent value read and validated just once. > > > > Also, as pointed out by Andi Kleen, we need to make sure that entire EL= F > > note is within a page bounds, so move the overflow check up and add an > > extra note_size boundaries validation. > > > > Fixes tag below points to the code that moved this code into > > lib/buildid.c, and then subsequently was used in perf subsystem, making > > this code exposed to perf_event_open() users in v5.12+. > > > > Cc: stable@vger.kernel.org > > Reviewed-by: Eduard Zingerman > > Reviewed-by: Jann Horn > > Suggested-by: Andi Kleen > > Fixes: bd7525dacd7e ("bpf: Move stack_map_get_build_id into lib") > > Signed-off-by: Andrii Nakryiko > > --- > > lib/buildid.c | 76 +++++++++++++++++++++++++++++---------------------- > > 1 file changed, 44 insertions(+), 32 deletions(-) > > > > diff --git a/lib/buildid.c b/lib/buildid.c > > index e02b5507418b..26007cc99a38 100644 > > --- a/lib/buildid.c > > +++ b/lib/buildid.c > > @@ -18,31 +18,37 @@ static int parse_build_id_buf(unsigned char *build_= id, > > const void *note_start, > > Elf32_Word note_size) > > { > > - Elf32_Word note_offs =3D 0, new_offs; > > - > > - while (note_offs + sizeof(Elf32_Nhdr) < note_size) { > > - Elf32_Nhdr *nhdr =3D (Elf32_Nhdr *)(note_start + note_off= s); > > + const char note_name[] =3D "GNU"; > > + const size_t note_name_sz =3D sizeof(note_name); > > + u64 note_off =3D 0, new_off, name_sz, desc_sz; > > + const char *data; > > + > > + while (note_off + sizeof(Elf32_Nhdr) < note_size && > > + note_off + sizeof(Elf32_Nhdr) > note_off /* overflow */) { > > + Elf32_Nhdr *nhdr =3D (Elf32_Nhdr *)(note_start + note_off= ); > > + > > + name_sz =3D READ_ONCE(nhdr->n_namesz); > > + desc_sz =3D READ_ONCE(nhdr->n_descsz); > > + > > + new_off =3D note_off + sizeof(Elf32_Nhdr); > > + if (check_add_overflow(new_off, ALIGN(name_sz, 4), &new_o= ff) || > > + check_add_overflow(new_off, ALIGN(desc_sz, 4), &new_o= ff) || > > + new_off > note_size) > > + break; > > > > if (nhdr->n_type =3D=3D BUILD_ID && > > - nhdr->n_namesz =3D=3D sizeof("GNU") && > > - !strcmp((char *)(nhdr + 1), "GNU") && > > - nhdr->n_descsz > 0 && > > - nhdr->n_descsz <=3D BUILD_ID_SIZE_MAX) { > > - memcpy(build_id, > > - note_start + note_offs + > > - ALIGN(sizeof("GNU"), 4) + sizeof(Elf32_Nhd= r), > > - nhdr->n_descsz); > > - memset(build_id + nhdr->n_descsz, 0, > > - BUILD_ID_SIZE_MAX - nhdr->n_descsz); > > + name_sz =3D=3D note_name_sz && > > + memcmp(nhdr + 1, note_name, note_name_sz) =3D=3D 0 && > > + desc_sz > 0 && desc_sz <=3D BUILD_ID_SIZE_MAX) { > > + data =3D note_start + note_off + ALIGN(note_name_= sz, 4); > > + memcpy(build_id, data, desc_sz); > > + memset(build_id + desc_sz, 0, BUILD_ID_SIZE_MAX -= desc_sz); > > if (size) > > - *size =3D nhdr->n_descsz; > > + *size =3D desc_sz; > > return 0; > > } > > hi, > this fix is causing stable kernels to return wrong build id, > the change below seems to fix that (based on 6.6 stable) > > if we agree on the fix I'll send it to all affected stable trees > > jirka > > > --- > The parse_build_id_buf does not account Elf32_Nhdr header size > when getting the build id data pointer and returns wrong build > id data as result. > > This is problem only stable trees that merged c83a80d8b84f fix, > the upstream build id code was refactored and returns proper > build id. > > Fixes: c83a80d8b84f ("lib/buildid: harden build ID parsing logic") > Signed-off-by: Jiri Olsa > --- > lib/buildid.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/buildid.c b/lib/buildid.c > index d3bc3d0528d5..9fc46366597e 100644 > --- a/lib/buildid.c > +++ b/lib/buildid.c > @@ -40,7 +40,7 @@ static int parse_build_id_buf(unsigned char *build_id, > name_sz =3D=3D note_name_sz && > memcmp(nhdr + 1, note_name, note_name_sz) =3D=3D 0 && > desc_sz > 0 && desc_sz <=3D BUILD_ID_SIZE_MAX) { > - data =3D note_start + note_off + ALIGN(note_name_= sz, 4); > + data =3D note_start + note_off + sizeof(Elf32_Nhd= r) + ALIGN(note_name_sz, 4); ah, my screw up, sorry. LGTM Acked-by: Andrii Nakryiko > memcpy(build_id, data, desc_sz); > memset(build_id + desc_sz, 0, BUILD_ID_SIZE_MAX -= desc_sz); > if (size) > -- > 2.47.0 >