Skip to content

Commit 12c5caf

Browse files
mfijalkoKernel Patches Daemon
authored andcommitted
xdp: introduce xdp_convert_skb_to_buff()
Currently, generic XDP hook uses xdp_rxq_info from netstack Rx queues which do not have its XDP memory model registered. There is a case when XDP program calls bpf_xdp_adjust_tail() BPF helper, which in turn releases underlying memory. This happens when it consumes enough amount of bytes and when XDP buffer has fragments. For this action the memory model knowledge passed to XDP program is crucial so that core can call suitable function for freeing/recycling the page. For netstack queues it defaults to MEM_TYPE_PAGE_SHARED (0) due to lack of mem model registration. The problem we're fixing here is when kernel copied the skb to new buffer backed by system's page_pool and XDP buffer is built around it. Then when bpf_xdp_adjust_tail() calls __xdp_return(), it acts incorrectly due to mem type not being set to MEM_TYPE_PAGE_POOL and causes a page leak. Pull out the existing code from bpf_prog_run_generic_xdp() that init/prepares xdp_buff onto new helper xdp_convert_skb_to_buff() and embed there rxq's mem_type initialization that is assigned to xdp_buff. Make it agnostic to current skb->data position. This problem was triggered by syzbot as well as AF_XDP test suite which is about to be integrated to BPF CI. Reported-by: [email protected] Closes: https://lore.kernel.org/netdev/[email protected]/ Fixes: e6d5dbd ("xdp: add multi-buff support for xdp running in generic mode") Tested-by: Ihor Solodrai <[email protected]> Co-developed-by: Octavian Purdila <[email protected]> Signed-off-by: Octavian Purdila <[email protected]> # whole analysis, testing, initiating a fix Signed-off-by: Maciej Fijalkowski <[email protected]> # commit msg and proposed more robust fix Reviewed-by: Toke Høiland-Jørgensen <[email protected]>
1 parent 87c66e9 commit 12c5caf

File tree

2 files changed

+29
-21
lines changed

2 files changed

+29
-21
lines changed

include/net/xdp.h

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,31 @@ struct sk_buff *xdp_build_skb_from_frame(struct xdp_frame *xdpf,
384384
struct net_device *dev);
385385
struct xdp_frame *xdpf_clone(struct xdp_frame *xdpf);
386386

387+
static inline
388+
void xdp_convert_skb_to_buff(struct sk_buff *skb, struct xdp_buff *xdp,
389+
struct xdp_rxq_info *xdp_rxq)
390+
{
391+
u32 frame_sz, pkt_len;
392+
393+
/* SKB "head" area always have tailroom for skb_shared_info */
394+
frame_sz = skb_end_pointer(skb) - skb->head;
395+
frame_sz += SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
396+
pkt_len = skb_tail_pointer(skb) - skb_mac_header(skb);
397+
398+
xdp_init_buff(xdp, frame_sz, xdp_rxq);
399+
xdp_prepare_buff(xdp, skb->head, skb->mac_header, pkt_len, true);
400+
401+
if (skb_is_nonlinear(skb)) {
402+
skb_shinfo(skb)->xdp_frags_size = skb->data_len;
403+
xdp_buff_set_frags_flag(xdp);
404+
} else {
405+
xdp_buff_clear_frags_flag(xdp);
406+
}
407+
408+
xdp->rxq->mem.type = page_pool_page_is_pp(virt_to_head_page(xdp->data)) ?
409+
MEM_TYPE_PAGE_POOL : MEM_TYPE_PAGE_SHARED;
410+
}
411+
387412
static inline
388413
void xdp_convert_frame_to_buff(const struct xdp_frame *frame,
389414
struct xdp_buff *xdp)

net/core/dev.c

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5320,35 +5320,18 @@ static struct netdev_rx_queue *netif_get_rxqueue(struct sk_buff *skb)
53205320
u32 bpf_prog_run_generic_xdp(struct sk_buff *skb, struct xdp_buff *xdp,
53215321
const struct bpf_prog *xdp_prog)
53225322
{
5323-
void *orig_data, *orig_data_end, *hard_start;
5323+
void *orig_data, *orig_data_end;
53245324
struct netdev_rx_queue *rxqueue;
53255325
bool orig_bcast, orig_host;
5326-
u32 mac_len, frame_sz;
5326+
u32 metalen, act, mac_len;
53275327
__be16 orig_eth_type;
53285328
struct ethhdr *eth;
5329-
u32 metalen, act;
53305329
int off;
53315330

5332-
/* The XDP program wants to see the packet starting at the MAC
5333-
* header.
5334-
*/
5331+
rxqueue = netif_get_rxqueue(skb);
53355332
mac_len = skb->data - skb_mac_header(skb);
5336-
hard_start = skb->data - skb_headroom(skb);
5337-
5338-
/* SKB "head" area always have tailroom for skb_shared_info */
5339-
frame_sz = (void *)skb_end_pointer(skb) - hard_start;
5340-
frame_sz += SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
53415333

5342-
rxqueue = netif_get_rxqueue(skb);
5343-
xdp_init_buff(xdp, frame_sz, &rxqueue->xdp_rxq);
5344-
xdp_prepare_buff(xdp, hard_start, skb_headroom(skb) - mac_len,
5345-
skb_headlen(skb) + mac_len, true);
5346-
if (skb_is_nonlinear(skb)) {
5347-
skb_shinfo(skb)->xdp_frags_size = skb->data_len;
5348-
xdp_buff_set_frags_flag(xdp);
5349-
} else {
5350-
xdp_buff_clear_frags_flag(xdp);
5351-
}
5334+
xdp_convert_skb_to_buff(skb, xdp, &rxqueue->xdp_rxq);
53525335

53535336
orig_data_end = xdp->data_end;
53545337
orig_data = xdp->data;

0 commit comments

Comments
 (0)