[lvc-project] [PATCH 5.10 004/317] r8169: Fix possible ring buffer corruption on fragmented Tx packets.

[Greg Kroah-Hartman]

On Thu, Jun 13, 2024 at 09:27:18PM +0100, Ken Milmore wrote:
> On 13/06/2024 18:21, Alexey Khoroshilov wrote:
> > On 13.06.2024 14:30, Greg Kroah-Hartman wrote:
> >> 5.10-stable review patch.  If anyone has any objections, please let me know.
> > 
> > The patch is cleanly applied to 5.10, but it leads to uninit value
> > access in rtl_tx_slots_avail().
> > 
> > 
> > 	unsigned int frags;
> > 	u32 opts[2];
> > 
> > 	txd_first = tp->TxDescArray + entry;
> > 
> > 	if (unlikely(!rtl_tx_slots_avail(tp, frags))) {
> >                                              ^^^^^ - USE OF UNINIT VALUE
> > 		if (net_ratelimit())
> > 			netdev_err(dev, "BUG! Tx Ring full when queue awake!\n");
> > 		goto err_stop_0;
> > 	}
> > 
> > 	opts[1] = rtl8169_tx_vlan_tag(skb);
> > 	opts[0] = 0;
> > 
> > 	if (!rtl_chip_supports_csum_v2(tp))
> > 		rtl8169_tso_csum_v1(skb, opts);
> > 	else if (!rtl8169_tso_csum_v2(tp, skb, opts))
> > 		goto err_dma_0;
> > 
> > 	if (unlikely(rtl8169_tx_map(tp, opts, skb_headlen(skb), skb->data,
> > 				    entry, false)))
> > 		goto err_dma_0;
> > 
> > 	txd_first = tp->TxDescArray + entry;
> > 
> > 	frags = skb_shinfo(skb)->nr_frags;
> >         ^^^^^^   - INITIALIZATION IS HERE AFTER THE PATCH
> > 
> > There is no such problem in upstream because rtl_tx_slots_avail() has no
> > nr_frags argument there.
> > 
> > 
> > Found by Linux Verification Center (linuxtesting.org) with SVACE.
> > 
> > --
> > Alexey Khoroshilov
> > Linux Verification Center, ISPRAS
> 
> Looks like the frags argument was removed in commit 83c317d7b36bb (r8169: remove nr_frags argument from rtl_tx_slots_avail), which first appears in linux-5.11.
> 
> I dare say it would be safe to replace
>  	if (unlikely(!rtl_tx_slots_avail(tp, frags))) {
> with
>  	if (unlikely(!rtl_tx_slots_avail(tp, MAX_SKB_FRAGS))) {
> 
> Best wait for Heiner to confirm though.

I'll just drop this commit for now, thanks.

greg k-h