Re: [PATCH net-next] virtio-net: fix use-after-free in page_to_skb()

[Eric Dumazet]

On 4/20/21 7:51 PM, Guenter Roeck wrote:

> 
> sh does indeed fail, with the same symptoms as before, but so far I was not
> able to track it down to a specific commit. The alpha failure is different,
> though. It is a NULL pointer access.
> 
> Anyway, testing ...
> 
> The patch below does indeed fix the problem I am seeing on sh.
> 
> ... and it does fix the alpha problem as well. Neat, though I don't really 
> understand
> what a NULL pointer access and an unaligned access have to do with each other.
> 
> Great catch, thanks!
> 
> Guenter
> 

Note that build_skb(), without an additional skb_reserve(skb, NET_IP_ALIGN)
can not possibly work on arches that care about alignments.

That is because we can not both align skb->data and skb_shinfo(skb)

So unless we change build_skb() to make sure to align skb_shinfo(),
a fix could be simply :

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 
8cd76037c72481200ea3e8429e9fdfec005dad85..9cbe9c1737649450e451e3c65f59f794d1bf34b0
 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -415,7 +415,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
 
        shinfo_size = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
 
-       if (len > GOOD_COPY_LEN && tailroom >= shinfo_size) {
+       if (!_NET_IP_ALIGN && len > GOOD_COPY_LEN && tailroom >= shinfo_size) {
                skb = build_skb(p, truesize);
                if (unlikely(!skb))
                        return NULL;