[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAEf4BzbHk10Tt0h38dt=HLi5U9_4BoWN5NNiwjXy_KoBe2j=SQ@mail.gmail.com>
Date: Thu, 16 Mar 2023 15:07:38 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Jiri Olsa <jolsa@...nel.org>
Cc: Alexei Starovoitov <ast@...nel.org>,
Andrii Nakryiko <andrii@...nel.org>,
Hao Luo <haoluo@...gle.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Alexander Viro <viro@...iv.linux.org.uk>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Matthew Wilcox <willy@...radead.org>, bpf@...r.kernel.org,
linux-mm@...ck.org, linux-kernel@...r.kernel.org,
linux-fsdevel@...r.kernel.org, linux-perf-users@...r.kernel.org,
Martin KaFai Lau <kafai@...com>,
Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
John Fastabend <john.fastabend@...il.com>,
KP Singh <kpsingh@...omium.org>,
Stanislav Fomichev <sdf@...gle.com>,
Daniel Borkmann <daniel@...earbox.net>,
Namhyung Kim <namhyung@...il.com>,
Dave Chinner <david@...morbit.com>
Subject: Re: [PATCHv3 bpf-next 1/9] mm: Store build id in file object
On Thu, Mar 16, 2023 at 10:02 AM Jiri Olsa <jolsa@...nel.org> wrote:
>
> Storing build id in file object for elf executable with build
> id defined. The build id is stored when file is mmaped.
>
> The build id object assignment to the file is locked with existing
> file->f_mapping semaphore.
>
> The f_build_id pointer points either build id object or carries
> the error the build id retrieval failed on.
>
> It's hidden behind new config option CONFIG_FILE_BUILD_ID.
>
> Signed-off-by: Jiri Olsa <jolsa@...nel.org>
> ---
> fs/file_table.c | 3 +++
> include/linux/buildid.h | 17 +++++++++++++++++
> include/linux/fs.h | 7 +++++++
> lib/buildid.c | 42 +++++++++++++++++++++++++++++++++++++++++
> mm/Kconfig | 9 +++++++++
> mm/mmap.c | 18 ++++++++++++++++++
> 6 files changed, 96 insertions(+)
>
> diff --git a/fs/file_table.c b/fs/file_table.c
> index 372653b92617..d72f72503268 100644
> --- a/fs/file_table.c
> +++ b/fs/file_table.c
> @@ -29,6 +29,7 @@
> #include <linux/ima.h>
> #include <linux/swap.h>
> #include <linux/kmemleak.h>
> +#include <linux/buildid.h>
>
> #include <linux/atomic.h>
>
> @@ -48,6 +49,7 @@ static void file_free_rcu(struct rcu_head *head)
> {
> struct file *f = container_of(head, struct file, f_rcuhead);
>
> + file_build_id_free(f);
> put_cred(f->f_cred);
> kmem_cache_free(filp_cachep, f);
> }
> @@ -413,6 +415,7 @@ void __init files_init(void)
> filp_cachep = kmem_cache_create("filp", sizeof(struct file), 0,
> SLAB_HWCACHE_ALIGN | SLAB_PANIC | SLAB_ACCOUNT, NULL);
> percpu_counter_init(&nr_files, 0, GFP_KERNEL);
> + build_id_init();
> }
>
> /*
> diff --git a/include/linux/buildid.h b/include/linux/buildid.h
> index 3b7a0ff4642f..b8b2e00420d6 100644
> --- a/include/linux/buildid.h
> +++ b/include/linux/buildid.h
> @@ -3,9 +3,15 @@
> #define _LINUX_BUILDID_H
>
> #include <linux/mm_types.h>
> +#include <linux/slab.h>
>
> #define BUILD_ID_SIZE_MAX 20
>
> +struct build_id {
> + u32 sz;
> + char data[BUILD_ID_SIZE_MAX];
> +};
> +
> int build_id_parse(struct vm_area_struct *vma, unsigned char *build_id,
> __u32 *size);
> int build_id_parse_buf(const void *buf, unsigned char *build_id, u32 buf_size);
> @@ -17,4 +23,15 @@ void init_vmlinux_build_id(void);
> static inline void init_vmlinux_build_id(void) { }
> #endif
>
> +#ifdef CONFIG_FILE_BUILD_ID
> +void __init build_id_init(void);
> +void build_id_free(struct build_id *bid);
> +void file_build_id_free(struct file *f);
> +void vma_read_build_id(struct vm_area_struct *vma, struct build_id **bidp);
> +#else
> +static inline void __init build_id_init(void) { }
> +static inline void build_id_free(struct build_id *bid) { }
> +static inline void file_build_id_free(struct file *f) { }
> +#endif /* CONFIG_FILE_BUILD_ID */
> +
> #endif
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index c85916e9f7db..ce03fd965cdb 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -977,6 +977,13 @@ struct file {
> struct address_space *f_mapping;
> errseq_t f_wb_err;
> errseq_t f_sb_err; /* for syncfs */
> +#ifdef CONFIG_FILE_BUILD_ID
> + /*
> + * Initialized when the file is mmaped (mmap_region),
> + * guarded by f_mapping lock.
> + */
> + struct build_id *f_build_id;
> +#endif
> } __randomize_layout
> __attribute__((aligned(4))); /* lest something weird decides that 2 is OK */
>
> diff --git a/lib/buildid.c b/lib/buildid.c
> index dfc62625cae4..04181c0b7c21 100644
> --- a/lib/buildid.c
> +++ b/lib/buildid.c
> @@ -5,6 +5,7 @@
> #include <linux/elf.h>
> #include <linux/kernel.h>
> #include <linux/pagemap.h>
> +#include <linux/slab.h>
>
> #define BUILD_ID 3
>
> @@ -189,3 +190,44 @@ void __init init_vmlinux_build_id(void)
> build_id_parse_buf(&__start_notes, vmlinux_build_id, size);
> }
> #endif
> +
> +#ifdef CONFIG_FILE_BUILD_ID
> +
> +/* SLAB cache for build_id structures */
> +static struct kmem_cache *build_id_cachep;
> +
> +void vma_read_build_id(struct vm_area_struct *vma, struct build_id **bidp)
this function clearly has a result to return, so why use void function
and out parameters instead of just returning `struct build_id *`?
> +{
> + struct build_id *bid = ERR_PTR(-ENOMEM);
> + int err;
> +
> + bid = kmem_cache_alloc(build_id_cachep, GFP_KERNEL);
> + if (!bid)
> + goto out;
> + err = build_id_parse(vma, bid->data, &bid->sz);
> + if (err) {
> + build_id_free(bid);
> + bid = ERR_PTR(err);
> + }
[...]
Powered by blists - more mailing lists