Re: [net-next PATCH v2 7/8] udp: Do not copy destructor if one is not present

[Willem de Bruijn]

On Fri, May 4, 2018 at 8:31 PM, Alexander Duyck
 wrote:
> From: Alexander Duyck 
>
> This patch makes it so that if a destructor is not present we avoid trying
> to update the skb socket or any reference counting that would be associated
> with the NULL socket and/or descriptor. By doing this we can support
> traffic coming from another namespace without any issues.
>
> Signed-off-by: Alexander Duyck 

Acked-by: Willem de Bruijn 

[net-next PATCH v2 7/8] udp: Do not copy destructor if one is not present

[Alexander Duyck]

From: Alexander Duyck 

This patch makes it so that if a destructor is not present we avoid trying
to update the skb socket or any reference counting that would be associated
with the NULL socket and/or descriptor. By doing this we can support
traffic coming from another namespace without any issues.

Signed-off-by: Alexander Duyck 
---

v2: Do not overwrite destructor if not sock_wfree as per Eric
Drop WARN_ON as per Eric

 net/ipv4/udp_offload.c |   21 ++---
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index fd94bbb369b2..e802f6331589 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -195,6 +195,7 @@ struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
unsigned int sum_truesize = 0;
struct udphdr *uh;
unsigned int mss;
+   bool copy_dtor;
__sum16 check;
__be16 newlen;
 
@@ -208,12 +209,14 @@ struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
skb_pull(gso_skb, sizeof(*uh));
 
/* clear destructor to avoid skb_segment assigning it to tail */
-   WARN_ON_ONCE(gso_skb->destructor != sock_wfree);
-   gso_skb->destructor = NULL;
+   copy_dtor = gso_skb->destructor == sock_wfree;
+   if (copy_dtor)
+   gso_skb->destructor = NULL;
 
segs = skb_segment(gso_skb, features);
if (unlikely(IS_ERR_OR_NULL(segs))) {
-   gso_skb->destructor = sock_wfree;
+   if (copy_dtor)
+   gso_skb->destructor = sock_wfree;
return segs;
}
 
@@ -234,9 +237,11 @@ struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
(__force u32)newlen));
 
for (;;) {
-   seg->destructor = sock_wfree;
-   seg->sk = sk;
-   sum_truesize += seg->truesize;
+   if (copy_dtor) {
+   seg->destructor = sock_wfree;
+   seg->sk = sk;
+   sum_truesize += seg->truesize;
+   }
 
if (!seg->next)
break;
@@ -268,7 +273,9 @@ struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
uh->check = gso_make_checksum(seg, ~check);
 
/* update refcount for the packet */
-   refcount_add(sum_truesize - gso_skb->truesize, &sk->sk_wmem_alloc);
+   if (copy_dtor)
+   refcount_add(sum_truesize - gso_skb->truesize,
+&sk->sk_wmem_alloc);
 out:
return segs;
 }