On Tue, Oct 27, 2020 at 06:03:13PM +0100, KP Singh wrote:
[ ... ]
> diff --git a/kernel/bpf/bpf_task_storage.c b/kernel/bpf/bpf_task_storage.c
> new file mode 100644
> index 000000000000..774140c458cc
> --- /dev/null
> +++ b/kernel/bpf/bpf_task_storage.c
> @@ -0,0 +1,327 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2019 Facebook
> + * Copyright 2020 Google LLC.
> + */
> +
> +#include "linux/pid.h"
> +#include "linux/sched.h"
> +#include <linux/rculist.h>
> +#include <linux/list.h>
> +#include <linux/hash.h>
> +#include <linux/types.h>
> +#include <linux/spinlock.h>
> +#include <linux/bpf.h>
> +#include <linux/bpf_local_storage.h>
> +#include <net/sock.h>
Is this required?
> +#include <uapi/linux/sock_diag.h>
> +#include <uapi/linux/btf.h>
> +#include <linux/bpf_lsm.h>
> +#include <linux/btf_ids.h>
> +#include <linux/fdtable.h>
> +
> +DEFINE_BPF_STORAGE_CACHE(task_cache);
> +
> +static struct bpf_local_storage __rcu **task_storage_ptr(void *owner)
> +{
> + struct task_struct *task = owner;
> + struct bpf_storage_blob *bsb;
> +
> + bsb = bpf_task(task);
> + if (!bsb)
> + return NULL;
> + return &bsb->storage;
> +}
> +
> +static struct bpf_local_storage_data *
> +task_storage_lookup(struct task_struct *task, struct bpf_map *map,
> + bool cacheit_lockit)
> +{
> + struct bpf_local_storage *task_storage;
> + struct bpf_local_storage_map *smap;
> + struct bpf_storage_blob *bsb;
> +
> + bsb = bpf_task(task);
> + if (!bsb)
> + return NULL;
> +
> + task_storage = rcu_dereference(bsb->storage);
> + if (!task_storage)
> + return NULL;
> +
> + smap = (struct bpf_local_storage_map *)map;
> + return bpf_local_storage_lookup(task_storage, smap, cacheit_lockit);
> +}
> +
[ ... ]
> +static void *bpf_pid_task_storage_lookup_elem(struct bpf_map *map, void *key)
> +{
> + struct bpf_local_storage_data *sdata;
> + struct task_struct *task;
> + struct pid *pid;
> + struct file *f;
> + int fd, err;
> +
> + fd = *(int *)key;
> + f = fget_raw(fd);
> + if (!f)
> + return ERR_PTR(-EBADF);
> +
> + if (f->f_op != &pidfd_fops) {
> + err = -EBADF;
> + goto out_fput;
> + }
> +
> + pid = get_pid(f->private_data);
n00b question. Is get_pid(f->private_data) required?
f->private_data could be freed while holding f->f_count?
> + task = get_pid_task(pid, PIDTYPE_PID);
Should put_task_struct() be called before returning?
> + if (!task || !task_storage_ptr(task)) {
"!task_storage_ptr(task)" is unnecessary, task_storage_lookup() should
have taken care of it.
> + err = -ENOENT;
> + goto out;
> + }
> +
> + sdata = task_storage_lookup(task, map, true);
> + put_pid(pid);
> + return sdata ? sdata->data : NULL;
> +out:
> + put_pid(pid);
> +out_fput:
> + fput(f);
> + return ERR_PTR(err);
> +}
> +
[ ... ]
> +static int task_storage_map_btf_id;
> +const struct bpf_map_ops task_storage_map_ops = {
> + .map_meta_equal = bpf_map_meta_equal,
> + .map_alloc_check = bpf_local_storage_map_alloc_check,
> + .map_alloc = task_storage_map_alloc,
> + .map_free = task_storage_map_free,
> + .map_get_next_key = notsupp_get_next_key,
> + .map_lookup_elem = bpf_pid_task_storage_lookup_elem,
> + .map_update_elem = bpf_pid_task_storage_update_elem,
> + .map_delete_elem = bpf_pid_task_storage_delete_elem,
Please exercise the syscall use cases also in the selftest.
> + .map_check_btf = bpf_local_storage_map_check_btf,
> + .map_btf_name = "bpf_local_storage_map",
> + .map_btf_id = &task_storage_map_btf_id,
> + .map_owner_storage_ptr = task_storage_ptr,
> +};
> +
