Re: [PATCH] AF_RAW: Augment raw_send_hdrinc to expand skb to fit iphdr->ihl

[Eric Dumazet <eric.dumazet@gmail.com>]

Neil Horman a écrit :
> Augment raw_send_hdrinc to correct for incorrect ip header length values
> 
> A series of oopses was reported to me recently.  Apparently when using AF_RAW
> sockets to send data to peers that were reachable via ipsec encapsulation,
> people could panic or BUG halt their systems.
> 
> I've tracked the problem down to user space sending an invalid ip header over an
> AF_RAW socket with IP_HDRINCL set to 1.
> 
> Basically what happens is that userspace sends down an ip frame that includes
> only the header (no data), but sets the ip header ihl value to a large number,
> one that is larger than the total amount of data passed to the sendmsg call.  In
> raw_send_hdrincl, we allocate an skb based on the size of the data in the msghdr
> that was passed in, but assume the data is all valid.  Later during ipsec
> encapsulation, xfrm4_tranport_output moves the entire frame back in the skbuff
> to provide headroom for the ipsec headers.  During this operation, the
> skb->transport_header is repointed to a spot computed by
> skb->network_header + the ip header length (ihl).  Since so little data was
> passed in relative to the value of ihl provided by the raw socket, we point
> transport header to an unknown location, resulting in various crashes.
> 
> So, what to do about this?  My first thought was to simply return -EINVAL, and
> let user space sort it out.  I'm still thinking that might be the best way, but
> I thought I'd try this first, just in case someone has reason to try to
> send such a bogus frame through the kernel.  This solution simply checks the
> value of ihl in raw_send_hdrinc and expands the skb to fit, filling the new
> space with  IPOPT_NOOP options.  I've confirmed that it fixes the crashes that
> were reported.
> 
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> 

Thanks a lot for this detailed info, I wish everything could be explained like this !

I believe we should drop the request, since padding it is not what was expected by user.