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

Message-ID: <CAG48ez3VuVQbbCCPRudOGq8jTVkhH17qe6vv7opuCghHAAd3Zw@mail.gmail.com>
Date: Mon, 29 Jul 2024 17:47:51 +0200
From: Jann Horn <jannh@...gle.com>
To: Andrii Nakryiko <andrii@...nel.org>
Cc: 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 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:

[...]
                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) {
[...]