On Thu, Sep 12, 2024 at 02:55:38PM +0800, Xuan Zhuo wrote:
> On Wed, 11 Sep 2024 07:28:36 -0400, "Michael S. Tsirkin" <mst@xxxxxxxxxx> 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.
If you stress test this for a long enough time, and with
debug enabled, you will see a crash.
> >
> > >
> > > --
> > > 2.32.0.3.g01195cf9f
> >
