Re: [RFC PATCH 2/4] trace: Allow kprobes to override livepatched functions

[Menglong Dong]

On 2026/4/2 21:20 Yafang Shao <laoar.shao@xxxxxxxxx> write:
> On Thu, Apr 2, 2026 at 8:48 PM Menglong Dong <menglong.dong@xxxxxxxxx> wrote:
> >
> > On 2026/4/2 17:26, Yafang Shao wrote:
> > > Introduce the ability for kprobes to override the return values of
> > > functions that have been livepatched. This functionality is guarded by the
> > > CONFIG_KPROBE_OVERRIDE_KLP_FUNC configuration option.
> >
> > Hi, Yafang. This is a interesting idea.
> >
[...]
> 
> +/* noclone to avoid bond_get_slave_hook.constprop.0 */
> +__attribute__((__noclone__, __noinline__))
> +int bond_get_slave_hook(struct sk_buff *skb, u32 hash, unsigned int count)
> +{
> +       return -1;
> +}

Hi, yafang.

I see what you mean now. So you want to allow BPF program override
the return of all the kernel functions in a KLP module.

I think the security problem is a big issue. Image that we have a KLP
in our environment. Any users can crash the kernel by hook a BPF
program on it with the calling of bpf_override_write().

What's more, this is a little weird for me. If we allow to use bpf_override_return()
for the kernel functions in a KLP, why not we allow it in a common kernel
module, as KLP is a kind of kernel module. Then, why not we allow to
use it for all the kernel functions?

Can we mark the "bond_get_slave_hook" with ALLOW_ERROR_INJECTION() in
your example? Then we can override its return directly. This is a more
reasonable for me. With ALLOW_ERROR_INJECTION(), we are telling people that
anyone can modify the return of this function safely.

WDYT?

BTW, this is a BPF modification, so maybe we can use "bpf: xxx" for the title
of this patch. Then, the BPF maintainers can notice this patch ;)

Thanks!
Menglong Dong

> 
>  static struct slave *bond_xmit_3ad_xor_slave_get(struct bonding *bond,
>                                                  struct sk_buff *skb,
>                                                  struct bond_up_slave *slaves)
>  {
>         struct slave *slave;
>         unsigned int count;
> +       int slave_idx;
>         u32 hash;
> 
>         hash = bond_xmit_hash(bond, skb);
> @@ -5188,6 +5198,13 @@ static struct slave
> *bond_xmit_3ad_xor_slave_get(struct bonding *bond,
>         if (unlikely(!count))
>                 return NULL;
> 
> +       /* Try BPF hook first - returns slave index directly */
> +       slave_idx = bond_get_slave_hook(skb, hash, count);
> +       /* If BPF hook returned valid slave index, use it */
> +       if (slave_idx >= 0 && slave_idx < count) {
> +               slave = slaves->arr[slave_idx];
> +               return slave;
> +       }
>         slave = slaves->arr[hash % count];
>         return slave;
>  }
> 
> - The BPF program
> 
> SEC("kprobe/bond_get_slave_hook")
> int BPF_KPROBE(slave_selector, struct sk_buff *skb, u32 hash, u32 count)
> {
>         unsigned short net_hdr_off;
>         unsigned char *head;
>         struct iphdr iph;
>         int *slave_idx;
>         __u32 daddr;
> 
>         __u16 proto = BPF_CORE_READ(skb, protocol);
>         if (proto != bpf_htons(0x0800))
>                 return 0;
> 
>         head = BPF_CORE_READ(skb, head);
>         net_hdr_off = BPF_CORE_READ(skb, network_header);
> 
>         if (bpf_probe_read_kernel(&iph, sizeof(iph), head + net_hdr_off) != 0)
>                 return 0;
> 
>         daddr = iph.daddr;
>         slave_idx = bpf_map_lookup_elem(&ip_slave_map, &daddr);
>         if (slave_idx) {
>                 int idx = *slave_idx;
> 
>                 if (idx >= 0 && idx < (int)count)
>                         bpf_override_return(ctx, idx);
>         }
>         return 0;
> }
> 
> >
> > BTW, if we allow the usage of bpf_override_return() on the KLP patched
> > function, we should allow the usage of BPF_MODIFY_RETURN on this
> > case too, right?
> 
> It's a possibility, but I haven't tested that specifically yet.
> 
> -- 
> Regards
> Yafang