[dpdk-dev] [PATCH v2] kni: fix possible kernel crash with va2pa

[Ferruh Yigit]

On 3/12/2019 9:22 AM, Yangchao Zhou wrote:
> va2pa depends on the physical address and virtual address offset of
> current mbuf. It may get the wrong physical address of next mbuf which
> allocated in another hugepage segment.
> 
> In rte_mempool_populate_default(), trying to allocate whole block of
> contiguous memory could be failed. Then, it would reserve memory in
> several memzones that have different physical address and virtual address
> offsets. The rte_mempool_populate_default() is used by
> rte_pktmbuf_pool_create().
> 
> Signed-off-by: Yangchao Zhou <zhouyates at gmail.com>
> ---
> v2: Add an explanation that causes this problem.
>     Use m->next to store physical address.

<...>

> @@ -481,7 +486,7 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni)
>  	uint32_t ret;
>  	uint32_t len;
>  	uint32_t i, num_rq, num_fq, num;
> -	struct rte_kni_mbuf *kva;
> +	struct rte_kni_mbuf *kva, *_kva;
>  	void *data_kva;
>  	struct sk_buff *skb;
>  	struct net_device *dev = kni->net_dev;
> @@ -545,8 +550,11 @@ kni_net_rx_lo_fifo_skb(struct kni_dev *kni)
>  				if (!kva->next)
>  					break;
>  
> -				kva = pa2kva(va2pa(kva->next, kva));
> +				_kva = kva;
> +				kva = pa2kva(kva->next);
>  				data_kva = kva2data_kva(kva);
> +				/* Convert physical address to virtual address */
> +				_kva->next = pa2va(_kva->next, kva);
>  			}
>  		}
>  

Also need to update 'kni_net_rx_lo_fifo()', at worst to update 'next' fields
because it fills 'kni->free_q', without conversion userspace will crash.

<...>

> @@ -550,7 +563,7 @@ rte_kni_tx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned num)
>  	unsigned int i;
>  
>  	for (i = 0; i < num; i++)
> -		phy_mbufs[i] = va2pa(mbufs[i]);
> +		phy_mbufs[i] = va2pa_all(mbufs[i]);
>  
>  	ret = kni_fifo_put(kni->rx_q, phy_mbufs, num);

There is a problem here.

When fifo 'kni->rx_q' is full, 'rte_kni_tx_burst' will send less mbuf than
requested, than the application needs to handle the remaining packages, most
probably will free them, but now some packages has physical address in their
'next' field, which will cause app to crash.

I don't know really how to solve this.
Perhaps getting free count from 'kni->rx_q' and only convert that much
(va2pa_all) to physical address can work, but I can't estimate performance
effect of it.