On Fri, Dec 13, 2024 at 3:30 PM Amery Hung <amery.hung@xxxxxxxxxxxxx> wrote:
>
> Add basic kfuncs for working on skb in qdisc.
>
> Both bpf_qdisc_skb_drop() and bpf_kfree_skb() can be used to release
> a reference to an skb. However, bpf_qdisc_skb_drop() can only be called
> in .enqueue where a to_free skb list is available from kernel to defer
> the release. bpf_kfree_skb() should be used elsewhere. It is also used
> in bpf_obj_free_fields() when cleaning up skb in maps and collections.
>
> bpf_skb_get_hash() returns the flow hash of an skb, which can be used
> to build flow-based queueing algorithms.
>
> Finally, allow users to create read-only dynptr via bpf_dynptr_from_skb().
>
> Signed-off-by: Amery Hung <amery.hung@xxxxxxxxxxxxx>
> ---
> net/sched/bpf_qdisc.c | 77 ++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 76 insertions(+), 1 deletion(-)
>
> diff --git a/net/sched/bpf_qdisc.c b/net/sched/bpf_qdisc.c
> index a2e2db29e5fc..28959424eab0 100644
> --- a/net/sched/bpf_qdisc.c
> +++ b/net/sched/bpf_qdisc.c
> @@ -106,6 +106,67 @@ static int bpf_qdisc_btf_struct_access(struct bpf_verifier_log *log,
> return 0;
> }
>
> +__bpf_kfunc_start_defs();
> +
> +/* bpf_skb_get_hash - Get the flow hash of an skb.
> + * @skb: The skb to get the flow hash from.
> + */
> +__bpf_kfunc u32 bpf_skb_get_hash(struct sk_buff *skb)
> +{
> + return skb_get_hash(skb);
> +}
> +
> +/* bpf_kfree_skb - Release an skb's reference and drop it immediately.
> + * @skb: The skb whose reference to be released and dropped.
> + */
> +__bpf_kfunc void bpf_kfree_skb(struct sk_buff *skb)
> +{
> + kfree_skb(skb);
> +}
> +
> +/* bpf_qdisc_skb_drop - Drop an skb by adding it to a deferred free list.
> + * @skb: The skb whose reference to be released and dropped.
> + * @to_free_list: The list of skbs to be dropped.
> + */
> +__bpf_kfunc void bpf_qdisc_skb_drop(struct sk_buff *skb,
> + struct bpf_sk_buff_ptr *to_free_list)
> +{
> + __qdisc_drop(skb, (struct sk_buff **)to_free_list);
> +}
> +
> +__bpf_kfunc_end_defs();
> +
> +#define BPF_QDISC_KFUNC_xxx \
> + BPF_QDISC_KFUNC(bpf_skb_get_hash, KF_TRUSTED_ARGS) \
> + BPF_QDISC_KFUNC(bpf_kfree_skb, KF_RELEASE) \
> + BPF_QDISC_KFUNC(bpf_qdisc_skb_drop, KF_RELEASE) \
> +
> +BTF_KFUNCS_START(bpf_qdisc_kfunc_ids)
> +#define BPF_QDISC_KFUNC(name, flag) BTF_ID_FLAGS(func, name, flag)
> +BPF_QDISC_KFUNC_xxx
> +#undef BPF_QDISC_KFUNC
> +BTF_ID_FLAGS(func, bpf_dynptr_from_skb, KF_TRUSTED_ARGS)
> +BTF_KFUNCS_END(bpf_qdisc_kfunc_ids)
> +
> +#define BPF_QDISC_KFUNC(name, _) BTF_ID_LIST_SINGLE(name##_ids, func, name)
> +BPF_QDISC_KFUNC_xxx
> +#undef BPF_QDISC_KFUNC
> +
> +static int bpf_qdisc_kfunc_filter(const struct bpf_prog *prog, u32 kfunc_id)
> +{
Here is a null pointer dereference since prog->aux->attach_func_name
is not populated yet during check_cfg(). I will add:
if (!btf_id_set8_contains(&bpf_qdisc_kfunc_ids, kfunc_id) ||
!prog->aux->attach_func_name)
return 0;
> + if (kfunc_id == bpf_qdisc_skb_drop_ids[0])
> + if (strcmp(prog->aux->attach_func_name, "enqueue"))
> + return -EACCES;
> +
> + return 0;
> +}
> +
> +static const struct btf_kfunc_id_set bpf_qdisc_kfunc_set = {
> + .owner = THIS_MODULE,
> + .set = &bpf_qdisc_kfunc_ids,
> + .filter = bpf_qdisc_kfunc_filter,
> +};
> +
> static const struct bpf_verifier_ops bpf_qdisc_verifier_ops = {
> .get_func_proto = bpf_qdisc_get_func_proto,
> .is_valid_access = bpf_qdisc_is_valid_access,
> @@ -209,6 +270,20 @@ static struct bpf_struct_ops bpf_Qdisc_ops = {
>
> static int __init bpf_qdisc_kfunc_init(void)
> {
> - return register_bpf_struct_ops(&bpf_Qdisc_ops, Qdisc_ops);
> + int ret;
> + const struct btf_id_dtor_kfunc skb_kfunc_dtors[] = {
> + {
> + .btf_id = bpf_sk_buff_ids[0],
> + .kfunc_btf_id = bpf_kfree_skb_ids[0]
> + },
> + };
> +
> + ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_STRUCT_OPS, &bpf_qdisc_kfunc_set);
> + ret = ret ?: register_btf_id_dtor_kfuncs(skb_kfunc_dtors,
> + ARRAY_SIZE(skb_kfunc_dtors),
> + THIS_MODULE);
> + ret = ret ?: register_bpf_struct_ops(&bpf_Qdisc_ops, Qdisc_ops);
> +
> + return ret;
> }
> late_initcall(bpf_qdisc_kfunc_init);
> --
> 2.20.1
>
