[IPV6,v2,0/4] ipv6: allocate enough headroom in ip6_finish_output2()

Recently Syzkaller found one more issue on RHEL7-based OpenVz kernels.
During its investigation I've found that upstream is affected too. 

TEE target send sbk with small headroom into another interface which requires
an increased headroom.

ipv4 handles this problem in ip_finish_output2() and creates new skb with enough headroom,
though ip6_finish_output2() lacks this logic.

Suzkaller created C reproducer, it can be found in v1 cover-letter.

v2 changes: 
 new helper was created and used in ip6_finish_output2 and in ip6_xmit()
 small refactoring in changed functions: commonly used dereferences was replaced by variables

ToDo:
 clarify proper name for helper,
 move it into proper place,  
 use it in other similar places:
   pptp_xmit
   vrf_finish_output
   ax25_transmit_buffer
   ax25_rt_build_path
   bpf_out_neigh_v6
   bpf_out_neigh_v4
   ip_finish_output2
   ip6_tnl_xmit
   ipip6_tunnel_xmit
   ip_vs_prepare_tunneled_skb

Vasily Averin (4):
  ipv6: allocate enough headroom in ip6_finish_output2()
  ipv6: use new helper skb_expand_head() in ip6_xmit()
  ipv6: ip6_finish_output2 refactoring
  ipv6: ip6_xmit refactoring

 net/ipv6/ip6_output.c | 89 ++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 59 insertions(+), 30 deletions(-)

currently if skb does not have enough headroom skb_realloc_headrom is called.
It is not optimal because it creates new skb.

Unlike skb_realloc_headroom, new helper pskb_realloc_headroom 
does not allocate a new skb if possible; 
copies skb->sk on new skb when as needed
and frees original skb in case of failures.

This helps to simplify ip[6]_finish_output2(), ip6_xmit() and a few other
functions in vrf, ax25 and bpf.

There are few other cases where this helper can be used but they require
an additional investigations. 

NB: patch "ipv6: use pskb_realloc_headroom in ip6_finish_output2" depends on 
patch "ipv6: allocate enough headroom in ip6_finish_output2()" submitted separately
https://lkml.org/lkml/2021/7/12/732

Vasily Averin (7):
  skbuff: introduce pskb_realloc_headroom()
  ipv6: use pskb_realloc_headroom in ip6_finish_output2
  ipv6: use pskb_realloc_headroom in ip6_xmit
  ipv4: use pskb_realloc_headroom in ip_finish_output2
  vrf: use pskb_realloc_headroom in vrf_finish_output
  ax25: use pskb_realloc_headroom
  bpf: use pskb_realloc_headroom in bpf_out_neigh_v4/6

 drivers/net/vrf.c      | 14 +++------
 include/linux/skbuff.h |  2 ++
 net/ax25/ax25_out.c    | 13 +++------
 net/ax25/ax25_route.c  | 13 +++------
 net/core/filter.c      | 22 +++-----------
 net/core/skbuff.c      | 41 ++++++++++++++++++++++++++
 net/ipv4/ip_output.c   | 12 ++------
 net/ipv6/ip6_output.c  | 78 ++++++++++++++++++--------------------------------
 8 files changed, 89 insertions(+), 106 deletions(-)

On Mon, 12 Jul 2021 16:26:44 +0300 Vasily Averin wrote:
>  /**

> + *	pskb_realloc_headroom - reallocate header of &sk_buff

> + *	@skb: buffer to reallocate

> + *	@headroom: needed headroom

> + *

> + *	Unlike skb_realloc_headroom, this one does not allocate a new skb

> + *	if possible; copies skb->sk to new skb as needed

> + *	and frees original scb in case of failures.

> + *

> + *	It expect increased headroom, and generates warning otherwise.

> + */

> +

> +struct sk_buff *pskb_realloc_headroom(struct sk_buff *skb, unsigned int headroom)


I saw you asked about naming in a different sub-thread, what do you
mean by "'pskb_expand_head' have different semantic"? AFAIU the 'p'
in pskb stands for "private", meaning not shared. In fact
skb_realloc_headroom() should really be pskb... but it predates the 
'pskb' naming pattern by quite a while. Long story short
skb_expand_head() seems like a good name. With the current patch
pskb_realloc_headroom() vs skb_realloc_headroom() would give people
exactly the opposite intuition of what the code does.

On 7/12/21 8:53 PM, Jakub Kicinski wrote:
> I saw you asked about naming in a different sub-thread, what do you

> mean by "'pskb_expand_head' have different semantic"? AFAIU the 'p'

> in pskb stands for "private", meaning not shared. In fact

> skb_realloc_headroom() should really be pskb... but it predates the 

> 'pskb' naming pattern by quite a while. Long story short

> skb_expand_head() seems like a good name. With the current patch

> pskb_realloc_headroom() vs skb_realloc_headroom() would give people

> exactly the opposite intuition of what the code does.


Thank you for feedback,
I'll change helper name back to skb_expand_head() in next patch version.

	Vasily Averin