Subject: Re: [PATCH net-next 03/13] virtio_ring: packed: harden dma unmap for indirect

From: "Michael S. Tsirkin" <[email-masked]>
Date: Wed, 11 Sep 2024 07:28:36 -0400

As gcc luckily noted:

On Tue, Aug 20, 2024 at 03:33:20PM +0800, Xuan Zhuo wrote:
> @@ -1617,23 +1617,24 @@ static void detach_buf_packed(struct vring_virtqueue *vq,
>  	}
>  
>  	if (vq->indirect) {
> +		struct vring_desc_extra *extra;
>  		u32 len;
>  
>  		/* Free the indirect table, if any, now that it's unmapped. */
> -		desc = state->indir_desc;
> -		if (!desc)

desc is no longer initialized here

> +		extra = state->indir;
> +		if (!extra)
>  			return;
>  
>  		if (vring_need_unmap_buffer(vq)) {
>  			len = vq->packed.desc_extra[id].len;
>  			for (i = 0; i < len / sizeof(struct vring_packed_desc);
>  					i++)
> -				vring_unmap_desc_packed(vq, &desc[i]);
> +				vring_unmap_extra_packed(vq, &extra[i]);
>  		}
>  		kfree(desc);

but freed here

> -		state->indir_desc = NULL;
> +		state->indir = NULL;
>  	} else if (ctx) {
> -		*ctx = state->indir_desc;
> +		*ctx = state->indir;
>  	}
>  }

It seems unlikely this was always 0 on all paths with even
a small amount of stress, so now I question how this was tested.
Besides, do not ignore compiler warnings, and do not tweak code
to just make compiler shut up - they are your friend.

> -- 
> 2.32.0.3.g01195cf9f