Re: [PATCH] mt76: fix frag length allocation for usb

[Lorenzo Bianconi <lorenzo.bianconi@redhat.com>]

> This is correct fix for c12128ce44b0 ("mt76: use a per rx queue page
> fragment cache"). We use wrong length when we allocate segments for
> MCU transmissions, which require bigger segment size than e->buf_size.
> 
> Commit 481bb0432414 ("mt76: usb: make rx page_frag_cache access atomic")
> partially solved the problem or actually mask it by changing
> mt76u_mcu_init_rx() and mt76u_alloc_queues() sequence, so e->buf_size
> become non zero any longer, but still not big enough to handle MCU data.

Hi Stanislaw,

I agree that we should use len in page_frag_alloc() instead of q->buf_size, so

Acked-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>

but reviewing the code I guess the real issue is not q->buf_size (since it should
be bigger than MCU_RESP_URB_SIZE) but it is the sequence of calls in
mt76x0u_register_device() since mt76u_alloc_queues need to be called before
mt76u_mcu_init_rx()

Regards,
Lorenzo

> 
> Patch fixes memory corruption which can manifest itself as random,
> not easy to reproduce crashes, during mt76 driver load or unload.
> 
> Fixes: c12128ce44b0 ("mt76: use a per rx queue page fragment cache")
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
>  drivers/net/wireless/mediatek/mt76/usb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
> index de7785c4f6af..6b643ea701e3 100644
> --- a/drivers/net/wireless/mediatek/mt76/usb.c
> +++ b/drivers/net/wireless/mediatek/mt76/usb.c
> @@ -286,7 +286,7 @@ mt76u_fill_rx_sg(struct mt76_dev *dev, struct mt76u_buf *buf,
>  		void *data;
>  		int offset;
>  
> -		data = page_frag_alloc(&q->rx_page, q->buf_size, GFP_ATOMIC);
> +		data = page_frag_alloc(&q->rx_page, len, GFP_ATOMIC);
>  		if (!data)
>  			break;
>  
> -- 
> 2.7.5
>