| Message ID | 167542916933.1167230.1244118780145312645.stgit@firesoul (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 9dde0cd3b10f63bc4100ebadc7e32275baabfa68 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,V2] net: introduce skb_poison_list and use in kfree_skb_list | expand |
Hello: This patch was applied to netdev/net-next.git (master) by David S. Miller <davem@davemloft.net>: On Fri, 03 Feb 2023 13:59:29 +0100 you wrote: > First user of skb_poison_list is in kfree_skb_list_reason, to catch bugs > earlier like introduced in commit eedade12f4cb ("net: kfree_skb_list use > kmem_cache_free_bulk"). For completeness mentioned bug have been fixed in > commit f72ff8b81ebc ("net: fix kfree_skb_list use of skb_mark_not_on_list"). > > In case of a bug like mentioned commit we would have seen OOPS with: > general protection fault, probably for non-canonical address 0xdead000000000870 > And content of one the registers e.g. R13: dead000000000800 > > [...] Here is the summary with links: - [net-next,V2] net: introduce skb_poison_list and use in kfree_skb_list https://git.kernel.org/netdev/net-next/c/9dde0cd3b10f You are awesome, thank you!
diff --git a/include/linux/poison.h b/include/linux/poison.h index 2d3249eb0e62..2823f90fdab4 100644 --- a/include/linux/poison.h +++ b/include/linux/poison.h @@ -81,6 +81,9 @@ /********** net/core/page_pool.c **********/ #define PP_SIGNATURE (0x40 + POISON_POINTER_DELTA) +/********** net/core/skbuff.c **********/ +#define SKB_LIST_POISON_NEXT ((void *)(0x800 + POISON_POINTER_DELTA)) + /********** kernel/bpf/ **********/ #define BPF_PTR_POISON ((void *)(0xeB9FUL + POISON_POINTER_DELTA)) diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index 5ba12185f43e..1fa95b916342 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -1738,6 +1738,13 @@ static inline void skb_mark_not_on_list(struct sk_buff *skb) skb->next = NULL; } +static inline void skb_poison_list(struct sk_buff *skb) +{ +#ifdef CONFIG_DEBUG_NET + skb->next = SKB_LIST_POISON_NEXT; +#endif +} + /* Iterate through singly-linked GSO fragments of an skb. */ #define skb_list_walk_safe(first, skb, next_skb) \ for ((skb) = (first), (next_skb) = (skb) ? (skb)->next : NULL; (skb); \ diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 44a19805c355..624e9e4ec116 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -1000,8 +1000,10 @@ kfree_skb_list_reason(struct sk_buff *segs, enum skb_drop_reason reason) while (segs) { struct sk_buff *next = segs->next; - if (__kfree_skb_reason(segs, reason)) + if (__kfree_skb_reason(segs, reason)) { + skb_poison_list(segs); kfree_skb_add_bulk(segs, &sa, reason); + } segs = next; }
First user of skb_poison_list is in kfree_skb_list_reason, to catch bugs earlier like introduced in commit eedade12f4cb ("net: kfree_skb_list use kmem_cache_free_bulk"). For completeness mentioned bug have been fixed in commit f72ff8b81ebc ("net: fix kfree_skb_list use of skb_mark_not_on_list"). In case of a bug like mentioned commit we would have seen OOPS with: general protection fault, probably for non-canonical address 0xdead000000000870 And content of one the registers e.g. R13: dead000000000800 In this case skb->len is at offset 112 bytes (0x70) why fault happens at 0x800+0x70 = 0x870 Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> --- include/linux/poison.h | 3 +++ include/linux/skbuff.h | 7 +++++++ net/core/skbuff.c | 4 +++- 3 files changed, 13 insertions(+), 1 deletion(-)