linux-kernel - Re: [PATCH] virtio: acknowledge all features before access

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87mtjuv8od.fsf@redhat.com>
Date:   Mon, 17 Jan 2022 13:38:42 +0100
From:   Cornelia Huck <cohuck@...hat.com>
To:     "Michael S. Tsirkin" <mst@...hat.com>,
        Jason Wang <jasowang@...hat.com>
Cc:     linux-kernel@...r.kernel.org, stable@...r.kernel.org,
        Halil Pasic <pasic@...ux.ibm.com>,
        virtualization@...ts.linux-foundation.org
Subject: Re: [PATCH] virtio: acknowledge all features before access

On Mon, Jan 17 2022, "Michael S. Tsirkin" <mst@...hat.com> wrote:

> On Mon, Jan 17, 2022 at 02:31:49PM +0800, Jason Wang wrote:
>> 
>> 在 2022/1/15 上午4:09, Michael S. Tsirkin 写道:
>> > @@ -495,6 +494,10 @@ int virtio_device_restore(struct virtio_device *dev)
>> >   	/* We have a driver! */
>> >   	virtio_add_status(dev, VIRTIO_CONFIG_S_DRIVER);
>> > +	ret = dev->config->finalize_features(dev);
>> > +	if (ret)
>> > +		goto err;
>> 
>> 
>> Is this part of code related?
>> 
>> Thanks
>> 
>
> Yes. virtio_finalize_features no longer calls dev->config->finalize_features.
>
> I think the dev->config->finalize_features callback is actually
> a misnomer now, it just sends the features to device,
> finalize is FEATURES_OK. Renaming that is a bigger
> patch though, and I'd like this one to be cherry-pickable
> to stable.

Do we want to add a comment before the calls to ->finalize_features()
(/* write features to device */) and adapt the comment in virtio_ring.h?
Should still be stable-friendly, and giving the callback a better name
can be a follow-up patch.

>
>> > +
>> >   	ret = virtio_finalize_features(dev);
>> >   	if (ret)
>> >   		goto err;