linux-kernel - Re: [PATCH net] virtio-net: reject small vring sizes

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: <20230423031308-mutt-send-email-mst@kernel.org>
Date:   Sun, 23 Apr 2023 03:19:15 -0400
From:   "Michael S. Tsirkin" <mst@...hat.com>
To:     Alvaro Karsz <alvaro.karsz@...id-run.com>
Cc:     Jason Wang <jasowang@...hat.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "virtualization@...ts.linux-foundation.org" 
        <virtualization@...ts.linux-foundation.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net] virtio-net: reject small vring sizes

On Sun, Apr 23, 2023 at 06:51:46AM +0000, Alvaro Karsz wrote:
> > Yes that makes sense, it's architetural. We can disable ctrl vq though.
> 
> The problem here is that we know the vring size after calling virtnet_find_vqs, so the number of VQs already includes the control VQ.
> 
> Actually, many variables/settings that are initialized before we call virtnet_find_vqs may need modifications if we use small vrings.
> For example has_rss_hash_report, has_rss, hdr_len etc..
> 
> We could have a fixup function to fix everything after we discover that we are using small vrings, but, honestly, I think that this will be hard to maintain in the future, and I don't like this approach much.
> 
> The ideal thing will be to discover if we use small vrings in probe's beginning.
> 
> I'm looking for a way at the moment.

Hmm. I was wrong. There is no way to disable CVQ feature bit.

1. Reset the device.
2. Set the ACKNOWLEDGE status bit: the guest OS has notice the device.
3. Set the DRIVER status bit: the guest OS knows how to drive the device.
4. Read device feature bits, and write the subset of feature bits understood by the OS and driver to the
device. During this step the driver MAY read (but MUST NOT write) the device-specific configuration
fields to check that it can support the device before accepting it.
5. Set the FEATURES_OK status bit. The driver MUST NOT accept new feature bits after this step.
6. Re-read device status to ensure the FEATURES_OK bit is still set: otherwise, the device does not
support our subset of features and the device is unusable.
7. Perform device-specific setup, including discovery of virtqueues for the device, optional per-bus setup,
reading and possibly writing the device’s virtio configuration space, and population of virtqueues.
8. Set the DRIVER_OK status bit. At this point the device is “live”.

So features are confirmed before find vqs.

The rest of stuff can probably just be moved to after find_vqs without
much pain.

So if cvq is too small we can either
- probe but avoid using cvq
or
- fail probe

-- 
MST