linux-kernel - Re: [PATCH v6 3/6] fs/procfs: add build ID fetching to PROCMAP

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] [day] [month] [year] [list]

Message-ID: <CAEf4Bza761YA=io2p_E8qSxuOxkkKFF7=RXnK2vDUE4eUdUmBw@mail.gmail.com>
Date: Mon, 29 Jul 2024 09:52:19 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Jann Horn <jannh@...gle.com>
Cc: Andrii Nakryiko <andrii@...nel.org>, linux-fsdevel@...r.kernel.org, brauner@...nel.org, 
	viro@...iv.linux.org.uk, akpm@...ux-foundation.org, 
	linux-kernel@...r.kernel.org, bpf@...r.kernel.org, gregkh@...uxfoundation.org, 
	linux-mm@...ck.org, liam.howlett@...cle.com, surenb@...gle.com, 
	rppt@...nel.org, adobriyan@...il.com
Subject: Re: [PATCH v6 3/6] fs/procfs: add build ID fetching to PROCMAP_QUERY API

On Mon, Jul 29, 2024 at 8:48 AM Jann Horn <jannh@...gle.com> wrote:
>
> On Thu, Jun 27, 2024 at 7:08 PM Andrii Nakryiko <andrii@...nel.org> wrote:
> > The need to get ELF build ID reliably is an important aspect when
> > dealing with profiling and stack trace symbolization, and
> > /proc/<pid>/maps textual representation doesn't help with this.
> [...]
> > @@ -539,6 +543,21 @@ static int do_procmap_query(struct proc_maps_private *priv, void __user *uarg)
> >                 }
> >         }
> >
> > +       if (karg.build_id_size) {
> > +               __u32 build_id_sz;
> > +
> > +               err = build_id_parse(vma, build_id_buf, &build_id_sz);
> > +               if (err) {
> > +                       karg.build_id_size = 0;
> > +               } else {
> > +                       if (karg.build_id_size < build_id_sz) {
> > +                               err = -ENAMETOOLONG;
> > +                               goto out;
> > +                       }
> > +                       karg.build_id_size = build_id_sz;
> > +               }
> > +       }
>
> The diff doesn't have enough context lines to see it here, but the two
> closing curly braces above are another copy of exactly the same code
> block from the preceding patch. The current state in mainline looks
> like this, with two repetitions of exactly the same block:

Yeah, you are right, thanks for the heads up! Seems like a rebase
screw up which duplicated build_id logic. It doesn't have any negative
effects besides doing the same work twice (if build ID parsing is
requested), but I'll definitely will send a fix to drop the
duplication.

>
> [...]
>                 karg.dev_minor = 0;
>                 karg.inode = 0;
>         }
>
>         if (karg.build_id_size) {
>                 __u32 build_id_sz;
>
>                 err = build_id_parse(vma, build_id_buf, &build_id_sz);
>                 if (err) {
>                         karg.build_id_size = 0;
>                 } else {
>                         if (karg.build_id_size < build_id_sz) {
>                                 err = -ENAMETOOLONG;
>                                 goto out;
>                         }
>                         karg.build_id_size = build_id_sz;
>                 }
>         }
>
>         if (karg.build_id_size) {
>                 __u32 build_id_sz;
>
>                 err = build_id_parse(vma, build_id_buf, &build_id_sz);
>                 if (err) {
>                         karg.build_id_size = 0;
>                 } else {
>                         if (karg.build_id_size < build_id_sz) {
>                                 err = -ENAMETOOLONG;
>                                 goto out;
>                         }
>                         karg.build_id_size = build_id_sz;
>                 }
>         }
>
>         if (karg.vma_name_size) {
> [...]