On 2026/4/2 21:20 Yafang Shao <[email protected]> write:
> On Thu, Apr 2, 2026 at 8:48 PM Menglong Dong <[email protected]> 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
