Re: [GIT PULL] vhost,virtio,vdpa: bugfixes

[Paul Moses]

On Thu, Mar 12, 2026 at 06:56:40AM +0000, Paul Moses wrote:
> > On Wed, Mar 11, 2026 at 08:00:45PM -0400, Michael S. Tsirkin wrote:
> > > On Wed, Mar 11, 2026 at 02:12:17PM +0000, Paul Moses wrote:
> > > > >
> > > > > Indeed, seems legit, thanks!
> > > > >
> > > > > The only thing is that I dislike two error handling paths here.
> > > > > So I wonder why isn't this as simple as fixing the double free
> > > > > directly:
> > > > >
> > > >
> > > > That works for me, should I send it out?
> > > >
> > > > Thanks,
> > > > Paul
> > >
> > >
> > > Please do.
> > >
> > > Signed-off-by: Michael S. Tsirkin <mst@xxxxxxxxxx>
> >
> > Oh and I forgot
> >
> > Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@xxxxxxxxxxxxx>
> >
> >
> > >
> > > --
> > > MST
> >
> >
>
> Considering that this pattern has occurred in multiple independent source files,
> maybe it is worth it to make intent more clear? What about this?
>
> >From c3b9431dc736cec60c0b288b13d9fa718341f46a Mon Sep 17 00:00:00 2001
> From: Paul Moses <p@xxxxxxx>
> Date: Tue, 10 Mar 2026 15:23:58 -0500
> Subject: [PATCH] vdpa: don't free reply skb after genlmsg_reply()
>
> genlmsg_reply() hands the reply skb to netlink, and
> netlink_unicast() consumes it on all return paths, whether the
> skb is queued successfully or freed on an error path.
>
> vdpa_nl_cmd_dev_config_get_doit() currently jumps to nlmsg_free(msg)
> after genlmsg_reply() fails, which can hit the same skb twice.
>
> Return the genlmsg_reply() error directly and keep nlmsg_free()
> only for pre-reply failures.
>
> Fixes: ad69dd0bf26b ("vdpa: Introduce query of device config layout")
> Cc: stable@xxxxxxxxxxxxxxx
>
> Signed-off-by: Paul Moses <p@xxxxxxx>

I like crediting people for their work.  Let's do this. Pls post as a
proper patch starting a new thread, so all 0day tooling catches this,
and I'll apply. I'll then apply a rework removing the duplication, on
top.  Thanks!


> ---
>  drivers/vdpa/vdpa.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> index 34874beb0152e..702d3a7772219 100644
> --- a/drivers/vdpa/vdpa.c
> +++ b/drivers/vdpa/vdpa.c
> @@ -1352,15 +1352,19 @@ static int vdpa_nl_cmd_dev_config_get_doit(struct sk_buff *skb, struct genl_info
>  	}
>  	err = vdpa_dev_config_fill(vdev, msg, info->snd_portid, info->snd_seq,
>  				   0, info->extack);
> -	if (!err)
> -		err = genlmsg_reply(msg, info);
> +	if (err)
> +		goto mdev_err;
> +
> +	put_device(dev);
> +	up_read(&vdpa_dev_lock);
> +
> +	return genlmsg_reply(msg, info);
>
>  mdev_err:
>  	put_device(dev);
>  dev_err:
>  	up_read(&vdpa_dev_lock);
> -	if (err)
> -		nlmsg_free(msg);
> +	nlmsg_free(msg);
>  	return err;
>  }
>
> --
> 2.53.GIT
>


It's obvious now your intention was always to diminish my 
contribution. This really only reflects negatively on you.

You or claude didn't even disassemble it. It's slop. My
patch is objectively superior both in human readability
and performance.