lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <9df5ed1d-ef55-9272-22fd-f2324dc3b5ba@huawei.com> Date: Fri, 3 Nov 2023 11:50:10 +0800 From: Yunsheng Lin <linyunsheng@...wei.com> To: "Li,Rongqing" <lirongqing@...du.com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org> Subject: Re: [PATCH 1/2][net-next] skbuff: move netlink_large_alloc_large_skb() to skbuff.c On 2023/11/2 20:09, Li,Rongqing wrote: >> -----Original Message----- >> From: Yunsheng Lin <linyunsheng@...wei.com> >> Sent: Thursday, November 2, 2023 7:02 PM >> To: Li,Rongqing <lirongqing@...du.com>; netdev@...r.kernel.org >> Subject: Re: [PATCH 1/2][net-next] skbuff: move netlink_large_alloc_large_skb() >> to skbuff.c >> >> On 2023/11/2 14:28, Li RongQing wrote: >>> move netlink_alloc_large_skb and netlink_skb_destructor to skbuff.c >>> and rename them more generic, so they can be used elsewhere large >>> non-contiguous physical memory is needed >>> >>> Signed-off-by: Li RongQing <lirongqing@...du.com> >>> --- >>> include/linux/skbuff.h | 3 +++ >>> net/core/skbuff.c | 40 >> ++++++++++++++++++++++++++++++++++++++++ >>> net/netlink/af_netlink.c | 41 >>> ++--------------------------------------- >>> 3 files changed, 45 insertions(+), 39 deletions(-) >>> >>> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index >>> 4174c4b..774a401 100644 >>> --- a/include/linux/skbuff.h >>> +++ b/include/linux/skbuff.h >>> @@ -5063,5 +5063,8 @@ static inline void skb_mark_for_recycle(struct >>> sk_buff *skb) ssize_t skb_splice_from_iter(struct sk_buff *skb, struct iov_iter >> *iter, >>> ssize_t maxsize, gfp_t gfp); >>> >>> + >>> +void large_skb_destructor(struct sk_buff *skb); struct sk_buff >>> +*alloc_large_skb(unsigned int size, int broadcast); >>> #endif /* __KERNEL__ */ >>> #endif /* _LINUX_SKBUFF_H */ >>> diff --git a/net/core/skbuff.c b/net/core/skbuff.c index >>> 4570705..20ffcd5 100644 >>> --- a/net/core/skbuff.c >>> +++ b/net/core/skbuff.c >>> @@ -6917,3 +6917,43 @@ ssize_t skb_splice_from_iter(struct sk_buff *skb, >> struct iov_iter *iter, >>> return spliced ?: ret; >>> } >>> EXPORT_SYMBOL(skb_splice_from_iter); >>> + >>> +void large_skb_destructor(struct sk_buff *skb) { >>> + if (is_vmalloc_addr(skb->head)) { >>> + if (!skb->cloned || >>> + !atomic_dec_return(&(skb_shinfo(skb)->dataref))) >>> + vfree(skb->head); >>> + >>> + skb->head = NULL; >> >> There seems to be an assumption that skb returned from >> netlink_alloc_large_skb() is not expecting the frag page for shinfo->frags*, as the >> above NULL setting will bypass most of the handling in skb_release_data(),then >> how can we ensure that the user is not breaking the assumption if we make it >> more generic? >> > > How about to add WARN_ON(skb_shinfo(skb)-> nr_frags) to find this condition > There is some other handling other than skb_shinfo(skb)-> nr_frags, such as zcopy, fraglist and pp_recycle handling, I am not sure if adding those check in the normal datapatch is worth it if netlink_alloc_large_skb() is only used in the nlmsg operations, which is less performance senstive. If there are other nlmsg operations that needs it too? if not, maybe we limit netlink_alloc_large_skb() in nlmsg if we can assume all nlmsg APIs dosen't break the above assumptionm, introducing something like vnlmsg_new() or only change nlmsg_new() to use netlink_alloc_large_skb(), so that all nlmsg users can make use of it. If there is more user making use of netlink_alloc_large_skb() in the future, we can make it usable outside of nlmsg. > > -Li RongQing >> >
Powered by blists - more mailing lists