Re: [PATCH bpf v2 1/2] bpf, xdp: clean head/meta when expanding it

On Sun, Mar 30, 2025 at 8:27 PM Jiayuan Chen jiayuan.chen@linux.dev wrote:

The device allocates an skb, it additionally allocates a prepad size (usually equal to NET_SKB_PAD or XDP_PACKET_HEADROOM) but leaves it uninitialized.

The bpf_xdp_adjust_head function moves skb->data forward, which allows users to access data belonging to other programs, posing a security risk.

Reported-by: syzbot+0e6ddb1ef80986bdfe64@syzkaller.appspotmail.com Closes: https://lore.kernel.org/all/00000000000067f65105edbd295d@google.com/T/ Signed-off-by: Jiayuan Chen jiayuan.chen@linux.dev

---

include/uapi/linux/bpf.h | 8 +++++--- net/core/filter.c | 5 ++++- tools/include/uapi/linux/bpf.h | 6 ++++-- 3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index defa5bb881f4..be01a848cbbf 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -2760,8 +2760,9 @@ union bpf_attr {

• long bpf_xdp_adjust_head(struct xdp_buff *xdp_md, int delta)

• Description
  

• •         Adjust (move) *xdp_md*\ **->data** by *delta* bytes. Note that
    

• •         it is possible to use a negative value for *delta*. This helper
    

• •         Adjust (move) *xdp_md*\ **->data** by *delta* bytes. Note that
    

• •         it is possible to use a negative value for *delta*. If *delta*
    

• •         is negative, the new header will be memset to zero. This helper
    

  •         can be used to prepare the packet for pushing or popping
    

  •         headers.
    

@@ -2989,7 +2990,8 @@ union bpf_attr {

• long bpf_xdp_adjust_meta(struct xdp_buff *xdp_md, int delta)

• Description
  

•         Adjust the address pointed by *xdp_md*\ **->data_meta** by
  

• •         *delta* (which can be positive or negative). Note that this
    

• •         *delta* (which can be positive or negative). If *delta* is
    

• •         negative, the new meta will be memset to zero. Note that this
    

  •         operation modifies the address stored in *xdp_md*\ **->data**,
    

  •         so the latter must be loaded only after the helper has been
    

  •         called.
    

diff --git a/net/core/filter.c b/net/core/filter.c index 46ae8eb7a03c..5f01d373b719 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -3947,6 +3947,8 @@ BPF_CALL_2(bpf_xdp_adjust_head, struct xdp_buff *, xdp, int, offset) if (metalen) memmove(xdp->data_meta + offset, xdp->data_meta, metalen);

•   if (offset < 0)
  

•           memset(data, 0, -offset);
    xdp->data_meta += offset;
    xdp->data = data;
  
  

@@ -4239,7 +4241,8 @@ BPF_CALL_2(bpf_xdp_adjust_meta, struct xdp_buff *, xdp, int, offset) return -EINVAL; if (unlikely(xdp_metalen_invalid(metalen))) return -EACCES;

•   if (offset < 0)
  

•           memset(meta, 0, -offset);
  

Let's make everyone pay a performance penalty to silence KMSAN warning?

I don't think it's a good trade off.

Soft nack.