[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1726124138.2346847-1-xuanzhuo@linux.alibaba.com>
Date: Thu, 12 Sep 2024 14:55:38 +0800
From: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>
To: "Michael S. Tsirkin" <mst@...hat.com>
Cc: netdev@...r.kernel.org,
Jason Wang <jasowang@...hat.com>,
Eugenio PĂ©rez <eperezma@...hat.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Jesper Dangaard Brouer <hawk@...nel.org>,
John Fastabend <john.fastabend@...il.com>,
virtualization@...ts.linux.dev,
bpf@...r.kernel.org
Subject: Re: [PATCH net-next 03/13] virtio_ring: packed: harden dma unmap for indirect
On Wed, 11 Sep 2024 07:28:36 -0400, "Michael S. Tsirkin" <mst@...hat.com> wrote:
> 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
Will fix.
>
> > + 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.
I agree.
Normally I do this by make W=12, but we have too many message,
so I missed this.
make W=12 drivers/net/virtio_net.o drivers/virtio/virtio_ring.o
If not W=12, then I did not get any warning message.
How do you get the message quickly?
Thanks.
>
> >
> > --
> > 2.32.0.3.g01195cf9f
>
Powered by blists - more mailing lists