* Re: [PATCH v2 1/4] virtio-mem: validate device-reported block size
From: Michael S. Tsirkin @ 2026-07-17 10:10 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: David Hildenbrand (Arm), Hari Mishal, Jason Wang, Xuan Zhuo,
Eugenio Pérez, virtualization, linux-kernel, elena.reshetova,
carlos.bilbao.osdev
In-Reply-To: <2026071746-deviation-clad-1712@gregkh>
On Fri, Jul 17, 2026 at 11:14:23AM +0200, Greg Kroah-Hartman wrote:
> On Fri, Jul 17, 2026 at 04:59:32AM -0400, Michael S. Tsirkin wrote:
> > On Fri, Jul 17, 2026 at 10:39:40AM +0200, David Hildenbrand (Arm) wrote:
> > > On 7/17/26 07:48, Michael S. Tsirkin wrote:
> > > > On Thu, Jul 16, 2026 at 05:59:05PM +0200, David Hildenbrand (Arm) wrote:
> > > >>> Or do we just always trust virtio mem devices explicitly?
> > > >>
> > > >> It's hard for me to understand where we draw the line, really.
> > > >>
> > > >> But maybe MST can clarify what we care about in virtio world where the
> > > >> hypervisor is fully in charge of the device,
> > > >
> > > > Generally:
> > > > - The guest is expected to whitelist drivers (most drivers have not
> > > > been audited).
> > >
> > > But even if you audited your driver, who makes sure that we consider all ways
> > > where the device could mess with us?
> >
> > A lot of this is up to a correct setup. For example, make sure all
> > filesystems are encrypted and refuse to mount unencrypted ones.
> >
> > > Something feels off here.
> > >
> > > Handling selected out-of-spec scenarios like this feels like a band-aid. Happy
> > > to be corrected.
> >
> > Well Documentation/security/snp-tdx-threat-model.rst puts it like this:
> > It is important to note
> > that this doesn’t imply that the host or VMM are intentionally
> > malicious, but that there exists a security value in having a small CoCo
> > VM TCB.
> >
> > and
> >
> > While traditionally the host has unlimited access to guest data and can
> > leverage this access to attack the guest, the CoCo systems mitigate such
> > attacks by adding security features like guest data confidentiality and
> > integrity protection.
> >
> >
> > now, when we are talking about "mitigation" it is indeed becoming a bit
> > murky.
> >
> >
> > For me, a rule of thumb I came up with is that if the validation happens
> > to also be helful for users e.g. to work around buggy devices,
> > or maybe because we feel failing gracefully is nice because this
> > will allow to later make use of this config and old drivers will
> > fail but at least not panic, then it is good to include.
>
> Why not do what USB does? Don't trust the device until AFTER probe()
> succeeds? All of the needed checking should happen before then, as that
> is a "slow path" so lots of validation and the like can happen at that
> point.
>
> After that, during the normal data paths, after the driver is bound,
> trust it all you want as attempting to validate every single packet is
> just going to be impossible.
>
> thanks,
>
> greg k-h
People do expect that data path validation at this point.
--
MST
^ permalink raw reply
* Re: [PATCH v2 1/4] virtio-mem: validate device-reported block size
From: Greg Kroah-Hartman @ 2026-07-17 10:15 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: David Hildenbrand (Arm), Hari Mishal, Jason Wang, Xuan Zhuo,
Eugenio Pérez, virtualization, linux-kernel, elena.reshetova,
carlos.bilbao.osdev
In-Reply-To: <20260717060822-mutt-send-email-mst@kernel.org>
On Fri, Jul 17, 2026 at 06:10:41AM -0400, Michael S. Tsirkin wrote:
> On Fri, Jul 17, 2026 at 11:14:23AM +0200, Greg Kroah-Hartman wrote:
> > On Fri, Jul 17, 2026 at 04:59:32AM -0400, Michael S. Tsirkin wrote:
> > > On Fri, Jul 17, 2026 at 10:39:40AM +0200, David Hildenbrand (Arm) wrote:
> > > > On 7/17/26 07:48, Michael S. Tsirkin wrote:
> > > > > On Thu, Jul 16, 2026 at 05:59:05PM +0200, David Hildenbrand (Arm) wrote:
> > > > >>> Or do we just always trust virtio mem devices explicitly?
> > > > >>
> > > > >> It's hard for me to understand where we draw the line, really.
> > > > >>
> > > > >> But maybe MST can clarify what we care about in virtio world where the
> > > > >> hypervisor is fully in charge of the device,
> > > > >
> > > > > Generally:
> > > > > - The guest is expected to whitelist drivers (most drivers have not
> > > > > been audited).
> > > >
> > > > But even if you audited your driver, who makes sure that we consider all ways
> > > > where the device could mess with us?
> > >
> > > A lot of this is up to a correct setup. For example, make sure all
> > > filesystems are encrypted and refuse to mount unencrypted ones.
> > >
> > > > Something feels off here.
> > > >
> > > > Handling selected out-of-spec scenarios like this feels like a band-aid. Happy
> > > > to be corrected.
> > >
> > > Well Documentation/security/snp-tdx-threat-model.rst puts it like this:
> > > It is important to note
> > > that this doesn’t imply that the host or VMM are intentionally
> > > malicious, but that there exists a security value in having a small CoCo
> > > VM TCB.
> > >
> > > and
> > >
> > > While traditionally the host has unlimited access to guest data and can
> > > leverage this access to attack the guest, the CoCo systems mitigate such
> > > attacks by adding security features like guest data confidentiality and
> > > integrity protection.
> > >
> > >
> > > now, when we are talking about "mitigation" it is indeed becoming a bit
> > > murky.
> > >
> > >
> > > For me, a rule of thumb I came up with is that if the validation happens
> > > to also be helful for users e.g. to work around buggy devices,
> > > or maybe because we feel failing gracefully is nice because this
> > > will allow to later make use of this config and old drivers will
> > > fail but at least not panic, then it is good to include.
> >
> > Why not do what USB does? Don't trust the device until AFTER probe()
> > succeeds? All of the needed checking should happen before then, as that
> > is a "slow path" so lots of validation and the like can happen at that
> > point.
> >
> > After that, during the normal data paths, after the driver is bound,
> > trust it all you want as attempting to validate every single packet is
> > just going to be impossible.
> >
> > thanks,
> >
> > greg k-h
>
> People do expect that data path validation at this point.
Ok, so you want this patch :)
And more, as you need to treat everything from the host as "untrusted",
and it must be "verified". Much like the proposed Rust patches we keep
slowly working on, which would make this much more obvious as to exactly
what needs to be verified, and where it happens.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v2 1/4] virtio-mem: validate device-reported block size
From: David Hildenbrand (Arm) @ 2026-07-17 10:21 UTC (permalink / raw)
To: Greg Kroah-Hartman, Michael S. Tsirkin
Cc: Hari Mishal, Jason Wang, Xuan Zhuo, Eugenio Pérez,
virtualization, linux-kernel, elena.reshetova,
carlos.bilbao.osdev
In-Reply-To: <2026071757-grout-composer-165d@gregkh>
On 7/17/26 12:15, Greg Kroah-Hartman wrote:
> On Fri, Jul 17, 2026 at 06:10:41AM -0400, Michael S. Tsirkin wrote:
>> On Fri, Jul 17, 2026 at 11:14:23AM +0200, Greg Kroah-Hartman wrote:
>>>
>>> Why not do what USB does? Don't trust the device until AFTER probe()
>>> succeeds? All of the needed checking should happen before then, as that
>>> is a "slow path" so lots of validation and the like can happen at that
>>> point.
>>>
>>> After that, during the normal data paths, after the driver is bound,
>>> trust it all you want as attempting to validate every single packet is
>>> just going to be impossible.
>>>
>>> thanks,
>>>
>>> greg k-h
>>
>> People do expect that data path validation at this point.
>
> Ok, so you want this patch :)
I fail to see the value of this patch given that there are plenty of other cases
the device can mess with us.
But sure, let's check for some conditions if it makes us feel warm and fluffy as
we audited a driver and it's now super safe, fine with me.
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH v2 1/4] virtio-mem: validate device-reported block size
From: Michael S. Tsirkin @ 2026-07-17 10:23 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: David Hildenbrand (Arm), Hari Mishal, Jason Wang, Xuan Zhuo,
Eugenio Pérez, virtualization, linux-kernel, elena.reshetova,
carlos.bilbao.osdev
In-Reply-To: <2026071757-grout-composer-165d@gregkh>
On Fri, Jul 17, 2026 at 12:15:09PM +0200, Greg Kroah-Hartman wrote:
> On Fri, Jul 17, 2026 at 06:10:41AM -0400, Michael S. Tsirkin wrote:
> > On Fri, Jul 17, 2026 at 11:14:23AM +0200, Greg Kroah-Hartman wrote:
> > > On Fri, Jul 17, 2026 at 04:59:32AM -0400, Michael S. Tsirkin wrote:
> > > > On Fri, Jul 17, 2026 at 10:39:40AM +0200, David Hildenbrand (Arm) wrote:
> > > > > On 7/17/26 07:48, Michael S. Tsirkin wrote:
> > > > > > On Thu, Jul 16, 2026 at 05:59:05PM +0200, David Hildenbrand (Arm) wrote:
> > > > > >>> Or do we just always trust virtio mem devices explicitly?
> > > > > >>
> > > > > >> It's hard for me to understand where we draw the line, really.
> > > > > >>
> > > > > >> But maybe MST can clarify what we care about in virtio world where the
> > > > > >> hypervisor is fully in charge of the device,
> > > > > >
> > > > > > Generally:
> > > > > > - The guest is expected to whitelist drivers (most drivers have not
> > > > > > been audited).
> > > > >
> > > > > But even if you audited your driver, who makes sure that we consider all ways
> > > > > where the device could mess with us?
> > > >
> > > > A lot of this is up to a correct setup. For example, make sure all
> > > > filesystems are encrypted and refuse to mount unencrypted ones.
> > > >
> > > > > Something feels off here.
> > > > >
> > > > > Handling selected out-of-spec scenarios like this feels like a band-aid. Happy
> > > > > to be corrected.
> > > >
> > > > Well Documentation/security/snp-tdx-threat-model.rst puts it like this:
> > > > It is important to note
> > > > that this doesn’t imply that the host or VMM are intentionally
> > > > malicious, but that there exists a security value in having a small CoCo
> > > > VM TCB.
> > > >
> > > > and
> > > >
> > > > While traditionally the host has unlimited access to guest data and can
> > > > leverage this access to attack the guest, the CoCo systems mitigate such
> > > > attacks by adding security features like guest data confidentiality and
> > > > integrity protection.
> > > >
> > > >
> > > > now, when we are talking about "mitigation" it is indeed becoming a bit
> > > > murky.
> > > >
> > > >
> > > > For me, a rule of thumb I came up with is that if the validation happens
> > > > to also be helful for users e.g. to work around buggy devices,
> > > > or maybe because we feel failing gracefully is nice because this
> > > > will allow to later make use of this config and old drivers will
> > > > fail but at least not panic, then it is good to include.
> > >
> > > Why not do what USB does? Don't trust the device until AFTER probe()
> > > succeeds? All of the needed checking should happen before then, as that
> > > is a "slow path" so lots of validation and the like can happen at that
> > > point.
> > >
> > > After that, during the normal data paths, after the driver is bound,
> > > trust it all you want as attempting to validate every single packet is
> > > just going to be impossible.
> > >
> > > thanks,
> > >
> > > greg k-h
> >
> > People do expect that data path validation at this point.
>
> Ok, so you want this patch :)
>
> And more, as you need to treat everything from the host as "untrusted",
> and it must be "verified".
Well. First it's not me) Second it's only specific configurations -
for example there's no short term plan to validate filesystem code, people
are expected to rely on encryption. The reasons have more to do
with the available manpower than anything else.
> Much like the proposed Rust patches we keep
> slowly working on, which would make this much more obvious as to exactly
> what needs to be verified, and where it happens.
>
> thanks,
>
> greg k-h
Or much like networking, except it's an old robust codebase.
--
MST
^ permalink raw reply
* Re: [PATCH v2 1/4] virtio-mem: validate device-reported block size
From: Michael S. Tsirkin @ 2026-07-17 10:28 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: Greg Kroah-Hartman, Hari Mishal, Jason Wang, Xuan Zhuo,
Eugenio Pérez, virtualization, linux-kernel, elena.reshetova,
carlos.bilbao.osdev
In-Reply-To: <5877a6ae-5a9c-4376-8297-f17695964db0@kernel.org>
On Fri, Jul 17, 2026 at 11:21:34AM +0100, David Hildenbrand (Arm) wrote:
> On 7/17/26 12:15, Greg Kroah-Hartman wrote:
> > On Fri, Jul 17, 2026 at 06:10:41AM -0400, Michael S. Tsirkin wrote:
> >> On Fri, Jul 17, 2026 at 11:14:23AM +0200, Greg Kroah-Hartman wrote:
> >>>
> >>> Why not do what USB does? Don't trust the device until AFTER probe()
> >>> succeeds? All of the needed checking should happen before then, as that
> >>> is a "slow path" so lots of validation and the like can happen at that
> >>> point.
> >>>
> >>> After that, during the normal data paths, after the driver is bound,
> >>> trust it all you want as attempting to validate every single packet is
> >>> just going to be impossible.
> >>>
> >>> thanks,
> >>>
> >>> greg k-h
> >>
> >> People do expect that data path validation at this point.
> >
> > Ok, so you want this patch :)
>
> I fail to see the value of this patch given that there are plenty of other cases
> the device can mess with us.
>
> But sure, let's check for some conditions if it makes us feel warm and fluffy as
> we audited a driver and it's now super safe, fine with me.
>
> --
> Cheers,
>
> David
I merely made some generic comments since I was asked to clarify how
virtio interacts with coco. As for the specific patch, it does not make
me feel fluffy. It lacks motivation.
A broken device confuses the kernel with no way to exploit that. Shrug. So?
Nothing to do with coco, apparently.
It's maybe reasonable as a debugging aid but if this is intended as such
let's make it clear in the commit log.
--
MST
^ permalink raw reply
* Re: [PATCH v2 1/4] virtio-mem: validate device-reported block size
From: Greg Kroah-Hartman @ 2026-07-17 10:44 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: Michael S. Tsirkin, Hari Mishal, Jason Wang, Xuan Zhuo,
Eugenio Pérez, virtualization, linux-kernel, elena.reshetova,
carlos.bilbao.osdev
In-Reply-To: <5877a6ae-5a9c-4376-8297-f17695964db0@kernel.org>
On Fri, Jul 17, 2026 at 11:21:34AM +0100, David Hildenbrand (Arm) wrote:
> On 7/17/26 12:15, Greg Kroah-Hartman wrote:
> > On Fri, Jul 17, 2026 at 06:10:41AM -0400, Michael S. Tsirkin wrote:
> >> On Fri, Jul 17, 2026 at 11:14:23AM +0200, Greg Kroah-Hartman wrote:
> >>>
> >>> Why not do what USB does? Don't trust the device until AFTER probe()
> >>> succeeds? All of the needed checking should happen before then, as that
> >>> is a "slow path" so lots of validation and the like can happen at that
> >>> point.
> >>>
> >>> After that, during the normal data paths, after the driver is bound,
> >>> trust it all you want as attempting to validate every single packet is
> >>> just going to be impossible.
> >>>
> >>> thanks,
> >>>
> >>> greg k-h
> >>
> >> People do expect that data path validation at this point.
> >
> > Ok, so you want this patch :)
>
> I fail to see the value of this patch given that there are plenty of other cases
> the device can mess with us.
How can the device mess with us?
> But sure, let's check for some conditions if it makes us feel warm and fluffy as
> we audited a driver and it's now super safe, fine with me.
I'm all for the folly of "it's an audited driver!" claims, but you all
need to decide either you do or you do not trust the device. Either way
is fine with me.
If you don't trust it, great, take patches that fix that. If you do
trust it, great, reject those types of patches.
But pick one please.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v2 1/4] virtio-mem: validate device-reported block size
From: Greg Kroah-Hartman @ 2026-07-17 10:46 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: David Hildenbrand (Arm), Hari Mishal, Jason Wang, Xuan Zhuo,
Eugenio Pérez, virtualization, linux-kernel, elena.reshetova,
carlos.bilbao.osdev
In-Reply-To: <20260717061901-mutt-send-email-mst@kernel.org>
On Fri, Jul 17, 2026 at 06:23:57AM -0400, Michael S. Tsirkin wrote:
> On Fri, Jul 17, 2026 at 12:15:09PM +0200, Greg Kroah-Hartman wrote:
> > On Fri, Jul 17, 2026 at 06:10:41AM -0400, Michael S. Tsirkin wrote:
> > > On Fri, Jul 17, 2026 at 11:14:23AM +0200, Greg Kroah-Hartman wrote:
> > > > On Fri, Jul 17, 2026 at 04:59:32AM -0400, Michael S. Tsirkin wrote:
> > > > > On Fri, Jul 17, 2026 at 10:39:40AM +0200, David Hildenbrand (Arm) wrote:
> > > > > > On 7/17/26 07:48, Michael S. Tsirkin wrote:
> > > > > > > On Thu, Jul 16, 2026 at 05:59:05PM +0200, David Hildenbrand (Arm) wrote:
> > > > > > >>> Or do we just always trust virtio mem devices explicitly?
> > > > > > >>
> > > > > > >> It's hard for me to understand where we draw the line, really.
> > > > > > >>
> > > > > > >> But maybe MST can clarify what we care about in virtio world where the
> > > > > > >> hypervisor is fully in charge of the device,
> > > > > > >
> > > > > > > Generally:
> > > > > > > - The guest is expected to whitelist drivers (most drivers have not
> > > > > > > been audited).
> > > > > >
> > > > > > But even if you audited your driver, who makes sure that we consider all ways
> > > > > > where the device could mess with us?
> > > > >
> > > > > A lot of this is up to a correct setup. For example, make sure all
> > > > > filesystems are encrypted and refuse to mount unencrypted ones.
> > > > >
> > > > > > Something feels off here.
> > > > > >
> > > > > > Handling selected out-of-spec scenarios like this feels like a band-aid. Happy
> > > > > > to be corrected.
> > > > >
> > > > > Well Documentation/security/snp-tdx-threat-model.rst puts it like this:
> > > > > It is important to note
> > > > > that this doesn’t imply that the host or VMM are intentionally
> > > > > malicious, but that there exists a security value in having a small CoCo
> > > > > VM TCB.
> > > > >
> > > > > and
> > > > >
> > > > > While traditionally the host has unlimited access to guest data and can
> > > > > leverage this access to attack the guest, the CoCo systems mitigate such
> > > > > attacks by adding security features like guest data confidentiality and
> > > > > integrity protection.
> > > > >
> > > > >
> > > > > now, when we are talking about "mitigation" it is indeed becoming a bit
> > > > > murky.
> > > > >
> > > > >
> > > > > For me, a rule of thumb I came up with is that if the validation happens
> > > > > to also be helful for users e.g. to work around buggy devices,
> > > > > or maybe because we feel failing gracefully is nice because this
> > > > > will allow to later make use of this config and old drivers will
> > > > > fail but at least not panic, then it is good to include.
> > > >
> > > > Why not do what USB does? Don't trust the device until AFTER probe()
> > > > succeeds? All of the needed checking should happen before then, as that
> > > > is a "slow path" so lots of validation and the like can happen at that
> > > > point.
> > > >
> > > > After that, during the normal data paths, after the driver is bound,
> > > > trust it all you want as attempting to validate every single packet is
> > > > just going to be impossible.
> > > >
> > > > thanks,
> > > >
> > > > greg k-h
> > >
> > > People do expect that data path validation at this point.
> >
> > Ok, so you want this patch :)
> >
> > And more, as you need to treat everything from the host as "untrusted",
> > and it must be "verified".
>
> Well. First it's not me) Second it's only specific configurations -
> for example there's no short term plan to validate filesystem code, people
> are expected to rely on encryption. The reasons have more to do
> with the available manpower than anything else.
Sure, but again, for subsystems, you have to define your threat model as
the LLMs are churning against the code base and coming up with lots of
crazy ideas if a device should or should not be trusted and spitting out
patches and reports like the ones that are in the first few patches of
this series.
So please, pick a model, let's document it, and go with that. I am
getting directly conflicting responses here.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v2 1/4] virtio-mem: validate device-reported block size
From: Michael S. Tsirkin @ 2026-07-17 10:52 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: David Hildenbrand (Arm), Hari Mishal, Jason Wang, Xuan Zhuo,
Eugenio Pérez, virtualization, linux-kernel, elena.reshetova,
carlos.bilbao.osdev
In-Reply-To: <2026071724-asleep-pedigree-ea54@gregkh>
On Fri, Jul 17, 2026 at 12:46:52PM +0200, Greg Kroah-Hartman wrote:
> On Fri, Jul 17, 2026 at 06:23:57AM -0400, Michael S. Tsirkin wrote:
> > On Fri, Jul 17, 2026 at 12:15:09PM +0200, Greg Kroah-Hartman wrote:
> > > On Fri, Jul 17, 2026 at 06:10:41AM -0400, Michael S. Tsirkin wrote:
> > > > On Fri, Jul 17, 2026 at 11:14:23AM +0200, Greg Kroah-Hartman wrote:
> > > > > On Fri, Jul 17, 2026 at 04:59:32AM -0400, Michael S. Tsirkin wrote:
> > > > > > On Fri, Jul 17, 2026 at 10:39:40AM +0200, David Hildenbrand (Arm) wrote:
> > > > > > > On 7/17/26 07:48, Michael S. Tsirkin wrote:
> > > > > > > > On Thu, Jul 16, 2026 at 05:59:05PM +0200, David Hildenbrand (Arm) wrote:
> > > > > > > >>> Or do we just always trust virtio mem devices explicitly?
> > > > > > > >>
> > > > > > > >> It's hard for me to understand where we draw the line, really.
> > > > > > > >>
> > > > > > > >> But maybe MST can clarify what we care about in virtio world where the
> > > > > > > >> hypervisor is fully in charge of the device,
> > > > > > > >
> > > > > > > > Generally:
> > > > > > > > - The guest is expected to whitelist drivers (most drivers have not
> > > > > > > > been audited).
> > > > > > >
> > > > > > > But even if you audited your driver, who makes sure that we consider all ways
> > > > > > > where the device could mess with us?
> > > > > >
> > > > > > A lot of this is up to a correct setup. For example, make sure all
> > > > > > filesystems are encrypted and refuse to mount unencrypted ones.
> > > > > >
> > > > > > > Something feels off here.
> > > > > > >
> > > > > > > Handling selected out-of-spec scenarios like this feels like a band-aid. Happy
> > > > > > > to be corrected.
> > > > > >
> > > > > > Well Documentation/security/snp-tdx-threat-model.rst puts it like this:
> > > > > > It is important to note
> > > > > > that this doesn’t imply that the host or VMM are intentionally
> > > > > > malicious, but that there exists a security value in having a small CoCo
> > > > > > VM TCB.
> > > > > >
> > > > > > and
> > > > > >
> > > > > > While traditionally the host has unlimited access to guest data and can
> > > > > > leverage this access to attack the guest, the CoCo systems mitigate such
> > > > > > attacks by adding security features like guest data confidentiality and
> > > > > > integrity protection.
> > > > > >
> > > > > >
> > > > > > now, when we are talking about "mitigation" it is indeed becoming a bit
> > > > > > murky.
> > > > > >
> > > > > >
> > > > > > For me, a rule of thumb I came up with is that if the validation happens
> > > > > > to also be helful for users e.g. to work around buggy devices,
> > > > > > or maybe because we feel failing gracefully is nice because this
> > > > > > will allow to later make use of this config and old drivers will
> > > > > > fail but at least not panic, then it is good to include.
> > > > >
> > > > > Why not do what USB does? Don't trust the device until AFTER probe()
> > > > > succeeds? All of the needed checking should happen before then, as that
> > > > > is a "slow path" so lots of validation and the like can happen at that
> > > > > point.
> > > > >
> > > > > After that, during the normal data paths, after the driver is bound,
> > > > > trust it all you want as attempting to validate every single packet is
> > > > > just going to be impossible.
> > > > >
> > > > > thanks,
> > > > >
> > > > > greg k-h
> > > >
> > > > People do expect that data path validation at this point.
> > >
> > > Ok, so you want this patch :)
> > >
> > > And more, as you need to treat everything from the host as "untrusted",
> > > and it must be "verified".
> >
> > Well. First it's not me) Second it's only specific configurations -
> > for example there's no short term plan to validate filesystem code, people
> > are expected to rely on encryption. The reasons have more to do
> > with the available manpower than anything else.
>
> Sure, but again, for subsystems, you have to define your threat model as
> the LLMs are churning against the code base and coming up with lots of
> crazy ideas if a device should or should not be trusted and spitting out
> patches and reports like the ones that are in the first few patches of
> this series.
>
> So please, pick a model, let's document it, and go with that. I am
> getting directly conflicting responses here.
>
> thanks,
>
> greg k-h
Supposed to be this one:
Documentation/security/snp-tdx-threat-model.rst
what is missing?
--
MST
^ permalink raw reply
* Re: [PATCH v2 1/4] virtio-mem: validate device-reported block size
From: David Hildenbrand (Arm) @ 2026-07-17 11:00 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Michael S. Tsirkin, Hari Mishal, Jason Wang, Xuan Zhuo,
Eugenio Pérez, virtualization, linux-kernel, elena.reshetova,
carlos.bilbao.osdev
In-Reply-To: <2026071717-unfiled-manned-3305@gregkh>
On 7/17/26 12:44, Greg Kroah-Hartman wrote:
> On Fri, Jul 17, 2026 at 11:21:34AM +0100, David Hildenbrand (Arm) wrote:
>> On 7/17/26 12:15, Greg Kroah-Hartman wrote:
>>>
>>> Ok, so you want this patch :)
>>
>> I fail to see the value of this patch given that there are plenty of other cases
>> the device can mess with us.
>
> How can the device mess with us?
I raised some examples already, like lying about which memory chunks it makes
available.
I guess it could also throw a random "requested_size" at us. It could make up a
wrong physical address region. I assume there is plenty more we'd have to check.
Again, I'm not saying that this couldn't/shouldn't be done, but it requires some
*real thought* about all possible things a device could do.
>
>> But sure, let's check for some conditions if it makes us feel warm and fluffy as
>> we audited a driver and it's now super safe, fine with me.
>
> I'm all for the folly of "it's an audited driver!" claims, but you all
> need to decide either you do or you do not trust the device. Either way
> is fine with me.
Well, exactly, that is what I am saying. I don't know what we care about. This
patch here feels incomplete and that's what grinds my gears. It doesn't
magically make us deal with malicious devices.
And I don't buy the story about "buggy virtio-mem devices that set
block_size==0", which doesn't make any sense in any possible reality.
>
> If you don't trust it, great, take patches that fix that. If you do
> trust it, great, reject those types of patches.
>
> But pick one please.
Yes, I'd expect that we have general virtio guifance. I only co-maintain some
virtio bits and have no idea about the expected trust model and when we would
consider a devices trusted.
And what it would take to get there.
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH v2 1/4] virtio-mem: validate device-reported block size
From: Greg Kroah-Hartman @ 2026-07-17 12:07 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: David Hildenbrand (Arm), Hari Mishal, Jason Wang, Xuan Zhuo,
Eugenio Pérez, virtualization, linux-kernel, elena.reshetova,
carlos.bilbao.osdev
In-Reply-To: <20260717065219-mutt-send-email-mst@kernel.org>
On Fri, Jul 17, 2026 at 06:52:46AM -0400, Michael S. Tsirkin wrote:
> On Fri, Jul 17, 2026 at 12:46:52PM +0200, Greg Kroah-Hartman wrote:
> > On Fri, Jul 17, 2026 at 06:23:57AM -0400, Michael S. Tsirkin wrote:
> > > On Fri, Jul 17, 2026 at 12:15:09PM +0200, Greg Kroah-Hartman wrote:
> > > > On Fri, Jul 17, 2026 at 06:10:41AM -0400, Michael S. Tsirkin wrote:
> > > > > On Fri, Jul 17, 2026 at 11:14:23AM +0200, Greg Kroah-Hartman wrote:
> > > > > > On Fri, Jul 17, 2026 at 04:59:32AM -0400, Michael S. Tsirkin wrote:
> > > > > > > On Fri, Jul 17, 2026 at 10:39:40AM +0200, David Hildenbrand (Arm) wrote:
> > > > > > > > On 7/17/26 07:48, Michael S. Tsirkin wrote:
> > > > > > > > > On Thu, Jul 16, 2026 at 05:59:05PM +0200, David Hildenbrand (Arm) wrote:
> > > > > > > > >>> Or do we just always trust virtio mem devices explicitly?
> > > > > > > > >>
> > > > > > > > >> It's hard for me to understand where we draw the line, really.
> > > > > > > > >>
> > > > > > > > >> But maybe MST can clarify what we care about in virtio world where the
> > > > > > > > >> hypervisor is fully in charge of the device,
> > > > > > > > >
> > > > > > > > > Generally:
> > > > > > > > > - The guest is expected to whitelist drivers (most drivers have not
> > > > > > > > > been audited).
> > > > > > > >
> > > > > > > > But even if you audited your driver, who makes sure that we consider all ways
> > > > > > > > where the device could mess with us?
> > > > > > >
> > > > > > > A lot of this is up to a correct setup. For example, make sure all
> > > > > > > filesystems are encrypted and refuse to mount unencrypted ones.
> > > > > > >
> > > > > > > > Something feels off here.
> > > > > > > >
> > > > > > > > Handling selected out-of-spec scenarios like this feels like a band-aid. Happy
> > > > > > > > to be corrected.
> > > > > > >
> > > > > > > Well Documentation/security/snp-tdx-threat-model.rst puts it like this:
> > > > > > > It is important to note
> > > > > > > that this doesn’t imply that the host or VMM are intentionally
> > > > > > > malicious, but that there exists a security value in having a small CoCo
> > > > > > > VM TCB.
> > > > > > >
> > > > > > > and
> > > > > > >
> > > > > > > While traditionally the host has unlimited access to guest data and can
> > > > > > > leverage this access to attack the guest, the CoCo systems mitigate such
> > > > > > > attacks by adding security features like guest data confidentiality and
> > > > > > > integrity protection.
> > > > > > >
> > > > > > >
> > > > > > > now, when we are talking about "mitigation" it is indeed becoming a bit
> > > > > > > murky.
> > > > > > >
> > > > > > >
> > > > > > > For me, a rule of thumb I came up with is that if the validation happens
> > > > > > > to also be helful for users e.g. to work around buggy devices,
> > > > > > > or maybe because we feel failing gracefully is nice because this
> > > > > > > will allow to later make use of this config and old drivers will
> > > > > > > fail but at least not panic, then it is good to include.
> > > > > >
> > > > > > Why not do what USB does? Don't trust the device until AFTER probe()
> > > > > > succeeds? All of the needed checking should happen before then, as that
> > > > > > is a "slow path" so lots of validation and the like can happen at that
> > > > > > point.
> > > > > >
> > > > > > After that, during the normal data paths, after the driver is bound,
> > > > > > trust it all you want as attempting to validate every single packet is
> > > > > > just going to be impossible.
> > > > > >
> > > > > > thanks,
> > > > > >
> > > > > > greg k-h
> > > > >
> > > > > People do expect that data path validation at this point.
> > > >
> > > > Ok, so you want this patch :)
> > > >
> > > > And more, as you need to treat everything from the host as "untrusted",
> > > > and it must be "verified".
> > >
> > > Well. First it's not me) Second it's only specific configurations -
> > > for example there's no short term plan to validate filesystem code, people
> > > are expected to rely on encryption. The reasons have more to do
> > > with the available manpower than anything else.
> >
> > Sure, but again, for subsystems, you have to define your threat model as
> > the LLMs are churning against the code base and coming up with lots of
> > crazy ideas if a device should or should not be trusted and spitting out
> > patches and reports like the ones that are in the first few patches of
> > this series.
> >
> > So please, pick a model, let's document it, and go with that. I am
> > getting directly conflicting responses here.
> >
> > thanks,
> >
> > greg k-h
>
> Supposed to be this one:
> Documentation/security/snp-tdx-threat-model.rst
>
> what is missing?
A policy decision that needs to be made. All that document does is
describe a bunch of different "threats" yet does not decide what to do
about them at all from what I can tell.
And that's just for one subset of the CoC world, right? Is that
something that all virtio drivers need/want to care about?
So I don't see a real answer to the "does Linux trust the host to give
you good data or not" question in that file, am I missing it?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v2 1/4] virtio-mem: validate device-reported block size
From: Michael S. Tsirkin @ 2026-07-17 13:08 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: David Hildenbrand (Arm), Hari Mishal, Jason Wang, Xuan Zhuo,
Eugenio Pérez, virtualization, linux-kernel, elena.reshetova,
carlos.bilbao.osdev
In-Reply-To: <2026071759-thermal-synopsis-7568@gregkh>
On Fri, Jul 17, 2026 at 02:07:50PM +0200, Greg Kroah-Hartman wrote:
> On Fri, Jul 17, 2026 at 06:52:46AM -0400, Michael S. Tsirkin wrote:
> > On Fri, Jul 17, 2026 at 12:46:52PM +0200, Greg Kroah-Hartman wrote:
> > > On Fri, Jul 17, 2026 at 06:23:57AM -0400, Michael S. Tsirkin wrote:
> > > > On Fri, Jul 17, 2026 at 12:15:09PM +0200, Greg Kroah-Hartman wrote:
> > > > > On Fri, Jul 17, 2026 at 06:10:41AM -0400, Michael S. Tsirkin wrote:
> > > > > > On Fri, Jul 17, 2026 at 11:14:23AM +0200, Greg Kroah-Hartman wrote:
> > > > > > > On Fri, Jul 17, 2026 at 04:59:32AM -0400, Michael S. Tsirkin wrote:
> > > > > > > > On Fri, Jul 17, 2026 at 10:39:40AM +0200, David Hildenbrand (Arm) wrote:
> > > > > > > > > On 7/17/26 07:48, Michael S. Tsirkin wrote:
> > > > > > > > > > On Thu, Jul 16, 2026 at 05:59:05PM +0200, David Hildenbrand (Arm) wrote:
> > > > > > > > > >>> Or do we just always trust virtio mem devices explicitly?
> > > > > > > > > >>
> > > > > > > > > >> It's hard for me to understand where we draw the line, really.
> > > > > > > > > >>
> > > > > > > > > >> But maybe MST can clarify what we care about in virtio world where the
> > > > > > > > > >> hypervisor is fully in charge of the device,
> > > > > > > > > >
> > > > > > > > > > Generally:
> > > > > > > > > > - The guest is expected to whitelist drivers (most drivers have not
> > > > > > > > > > been audited).
> > > > > > > > >
> > > > > > > > > But even if you audited your driver, who makes sure that we consider all ways
> > > > > > > > > where the device could mess with us?
> > > > > > > >
> > > > > > > > A lot of this is up to a correct setup. For example, make sure all
> > > > > > > > filesystems are encrypted and refuse to mount unencrypted ones.
> > > > > > > >
> > > > > > > > > Something feels off here.
> > > > > > > > >
> > > > > > > > > Handling selected out-of-spec scenarios like this feels like a band-aid. Happy
> > > > > > > > > to be corrected.
> > > > > > > >
> > > > > > > > Well Documentation/security/snp-tdx-threat-model.rst puts it like this:
> > > > > > > > It is important to note
> > > > > > > > that this doesn’t imply that the host or VMM are intentionally
> > > > > > > > malicious, but that there exists a security value in having a small CoCo
> > > > > > > > VM TCB.
> > > > > > > >
> > > > > > > > and
> > > > > > > >
> > > > > > > > While traditionally the host has unlimited access to guest data and can
> > > > > > > > leverage this access to attack the guest, the CoCo systems mitigate such
> > > > > > > > attacks by adding security features like guest data confidentiality and
> > > > > > > > integrity protection.
> > > > > > > >
> > > > > > > >
> > > > > > > > now, when we are talking about "mitigation" it is indeed becoming a bit
> > > > > > > > murky.
> > > > > > > >
> > > > > > > >
> > > > > > > > For me, a rule of thumb I came up with is that if the validation happens
> > > > > > > > to also be helful for users e.g. to work around buggy devices,
> > > > > > > > or maybe because we feel failing gracefully is nice because this
> > > > > > > > will allow to later make use of this config and old drivers will
> > > > > > > > fail but at least not panic, then it is good to include.
> > > > > > >
> > > > > > > Why not do what USB does? Don't trust the device until AFTER probe()
> > > > > > > succeeds? All of the needed checking should happen before then, as that
> > > > > > > is a "slow path" so lots of validation and the like can happen at that
> > > > > > > point.
> > > > > > >
> > > > > > > After that, during the normal data paths, after the driver is bound,
> > > > > > > trust it all you want as attempting to validate every single packet is
> > > > > > > just going to be impossible.
> > > > > > >
> > > > > > > thanks,
> > > > > > >
> > > > > > > greg k-h
> > > > > >
> > > > > > People do expect that data path validation at this point.
> > > > >
> > > > > Ok, so you want this patch :)
> > > > >
> > > > > And more, as you need to treat everything from the host as "untrusted",
> > > > > and it must be "verified".
> > > >
> > > > Well. First it's not me) Second it's only specific configurations -
> > > > for example there's no short term plan to validate filesystem code, people
> > > > are expected to rely on encryption. The reasons have more to do
> > > > with the available manpower than anything else.
> > >
> > > Sure, but again, for subsystems, you have to define your threat model as
> > > the LLMs are churning against the code base and coming up with lots of
> > > crazy ideas if a device should or should not be trusted and spitting out
> > > patches and reports like the ones that are in the first few patches of
> > > this series.
> > >
> > > So please, pick a model, let's document it, and go with that. I am
> > > getting directly conflicting responses here.
> > >
> > > thanks,
> > >
> > > greg k-h
> >
> > Supposed to be this one:
> > Documentation/security/snp-tdx-threat-model.rst
> >
> > what is missing?
>
> A policy decision that needs to be made. All that document does is
> describe a bunch of different "threats" yet does not decide what to do
> about them at all from what I can tell.
That would be this section I think:
The **Linux kernel CoCo VM security objectives** can be summarized as follows:
it does, indeed, not go into detail about how to interact, safely,
with untrusted entities. Does it really need to be spelled out?
> And that's just for one subset of the CoC world, right? Is that
> something that all virtio drivers need/want to care about?
What is missing, and what you seem to be asking for, is an opinionated
stance on which drivers we care about in this world?
True.
coco guys tried to annotate drivers at some point to do exactly that.
this was rejected upstream from the position that this is not
different from handling buggy hardware, and just to fix all drivers.
so it's up to users, and I guess for virtio the answer is yes
with some exceptions because we don't have a better answer right now.
> So I don't see a real answer to the "does Linux trust the host to give
> you good data or not" question in that file, am I missing it?
>
> thanks,
>
> greg k-h
This? Note the last sentence.
The **Linux CoCo VM attack surface** is any interface exposed from a CoCo
guest Linux kernel towards an untrusted host that is not covered by the
CoCo technology SW/HW protection. This includes any possible
side-channels, as well as transient execution side channels. Examples of
explicit (not side-channel) interfaces include accesses to port I/O, MMIO
and DMA interfaces, access to PCI configuration space, VMM-specific
hypercalls (towards Host-side VMM), access to shared memory pages,
interrupts allowed to be injected into the guest kernel by the host, as
well as CoCo technology-specific hypercalls, if present. Additionally, the
host in a CoCo system typically controls the process of creating a CoCo
guest: it has a method to load into a guest the firmware and bootloader
images, the kernel image together with the kernel command line. All of this
data should also be considered untrusted until its integrity and
authenticity is established via attestation.
^ permalink raw reply
* [PATCH] vhost-scsi: Prevent OOM from invalid protection SGL count
From: Jia Jia @ 2026-07-17 14:22 UTC (permalink / raw)
To: mst, jasowang, michael.christie
Cc: pbonzini, stefanha, eperezma, virtualization, kvm, netdev,
linux-kernel, stable
The protection SGL path passes the result of vhost_scsi_calc_sgls()
directly to sg_alloc_table_chained(). The helper returns a negative
errno when the iterator is invalid or the request exceeds the segment
limit. The negative errno is then treated as a very large unsigned count
and sends the request into the SGL allocation path with an invalid size.
Repeated malformed T10-PI submissions from a host-side application caused
memory usage to rise sharply. MemAvailable fell to about 200 MB, and PSI
full avg10 reached about 1.46. The OOM killer terminated several userspace
processes before the endpoint cleanup completed. The kernel log included:
[17036.451028] Out of memory: Killed process 2345 (systemd)
[17036.493325] Out of memory: Killed process 2349 (sd-pam)
[17078.265127] Out of memory: Killed process 1793 (networkd-dispat)
Return the calculation error before setting up the protection SGL. This
keeps the protection path consistent with the data SGL path and prevents
the invalid count from entering the allocation path.
Fixes: bca939d5bcd0 ("vhost-scsi: Dynamically allocate scatterlists")
Cc: stable@vger.kernel.org
Signed-off-by: Jia Jia <physicalmtea@gmail.com>
---
drivers/vhost/scsi.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 9a1253b9d..8486652fd 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -972,6 +972,9 @@ vhost_scsi_mapal(struct vhost_scsi *vs, struct vhost_scsi_cmd *cmd,
if (prot_bytes) {
sgl_count = vhost_scsi_calc_sgls(prot_iter, prot_bytes,
VHOST_SCSI_PREALLOC_PROT_SGLS);
+ if (sgl_count < 0)
+ return sgl_count;
+
cmd->prot_table.sgl = cmd->prot_sgl;
ret = sg_alloc_table_chained(&cmd->prot_table, sgl_count,
cmd->prot_table.sgl,
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v2 1/4] virtio-mem: validate device-reported block size
From: Greg Kroah-Hartman @ 2026-07-17 14:31 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: David Hildenbrand (Arm), Hari Mishal, Jason Wang, Xuan Zhuo,
Eugenio Pérez, virtualization, linux-kernel, elena.reshetova,
carlos.bilbao.osdev
In-Reply-To: <20260717085838-mutt-send-email-mst@kernel.org>
On Fri, Jul 17, 2026 at 09:08:23AM -0400, Michael S. Tsirkin wrote:
> On Fri, Jul 17, 2026 at 02:07:50PM +0200, Greg Kroah-Hartman wrote:
> > On Fri, Jul 17, 2026 at 06:52:46AM -0400, Michael S. Tsirkin wrote:
> > > On Fri, Jul 17, 2026 at 12:46:52PM +0200, Greg Kroah-Hartman wrote:
> > > > On Fri, Jul 17, 2026 at 06:23:57AM -0400, Michael S. Tsirkin wrote:
> > > > > On Fri, Jul 17, 2026 at 12:15:09PM +0200, Greg Kroah-Hartman wrote:
> > > > > > On Fri, Jul 17, 2026 at 06:10:41AM -0400, Michael S. Tsirkin wrote:
> > > > > > > On Fri, Jul 17, 2026 at 11:14:23AM +0200, Greg Kroah-Hartman wrote:
> > > > > > > > On Fri, Jul 17, 2026 at 04:59:32AM -0400, Michael S. Tsirkin wrote:
> > > > > > > > > On Fri, Jul 17, 2026 at 10:39:40AM +0200, David Hildenbrand (Arm) wrote:
> > > > > > > > > > On 7/17/26 07:48, Michael S. Tsirkin wrote:
> > > > > > > > > > > On Thu, Jul 16, 2026 at 05:59:05PM +0200, David Hildenbrand (Arm) wrote:
> > > > > > > > > > >>> Or do we just always trust virtio mem devices explicitly?
> > > > > > > > > > >>
> > > > > > > > > > >> It's hard for me to understand where we draw the line, really.
> > > > > > > > > > >>
> > > > > > > > > > >> But maybe MST can clarify what we care about in virtio world where the
> > > > > > > > > > >> hypervisor is fully in charge of the device,
> > > > > > > > > > >
> > > > > > > > > > > Generally:
> > > > > > > > > > > - The guest is expected to whitelist drivers (most drivers have not
> > > > > > > > > > > been audited).
> > > > > > > > > >
> > > > > > > > > > But even if you audited your driver, who makes sure that we consider all ways
> > > > > > > > > > where the device could mess with us?
> > > > > > > > >
> > > > > > > > > A lot of this is up to a correct setup. For example, make sure all
> > > > > > > > > filesystems are encrypted and refuse to mount unencrypted ones.
> > > > > > > > >
> > > > > > > > > > Something feels off here.
> > > > > > > > > >
> > > > > > > > > > Handling selected out-of-spec scenarios like this feels like a band-aid. Happy
> > > > > > > > > > to be corrected.
> > > > > > > > >
> > > > > > > > > Well Documentation/security/snp-tdx-threat-model.rst puts it like this:
> > > > > > > > > It is important to note
> > > > > > > > > that this doesn’t imply that the host or VMM are intentionally
> > > > > > > > > malicious, but that there exists a security value in having a small CoCo
> > > > > > > > > VM TCB.
> > > > > > > > >
> > > > > > > > > and
> > > > > > > > >
> > > > > > > > > While traditionally the host has unlimited access to guest data and can
> > > > > > > > > leverage this access to attack the guest, the CoCo systems mitigate such
> > > > > > > > > attacks by adding security features like guest data confidentiality and
> > > > > > > > > integrity protection.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > now, when we are talking about "mitigation" it is indeed becoming a bit
> > > > > > > > > murky.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > For me, a rule of thumb I came up with is that if the validation happens
> > > > > > > > > to also be helful for users e.g. to work around buggy devices,
> > > > > > > > > or maybe because we feel failing gracefully is nice because this
> > > > > > > > > will allow to later make use of this config and old drivers will
> > > > > > > > > fail but at least not panic, then it is good to include.
> > > > > > > >
> > > > > > > > Why not do what USB does? Don't trust the device until AFTER probe()
> > > > > > > > succeeds? All of the needed checking should happen before then, as that
> > > > > > > > is a "slow path" so lots of validation and the like can happen at that
> > > > > > > > point.
> > > > > > > >
> > > > > > > > After that, during the normal data paths, after the driver is bound,
> > > > > > > > trust it all you want as attempting to validate every single packet is
> > > > > > > > just going to be impossible.
> > > > > > > >
> > > > > > > > thanks,
> > > > > > > >
> > > > > > > > greg k-h
> > > > > > >
> > > > > > > People do expect that data path validation at this point.
> > > > > >
> > > > > > Ok, so you want this patch :)
> > > > > >
> > > > > > And more, as you need to treat everything from the host as "untrusted",
> > > > > > and it must be "verified".
> > > > >
> > > > > Well. First it's not me) Second it's only specific configurations -
> > > > > for example there's no short term plan to validate filesystem code, people
> > > > > are expected to rely on encryption. The reasons have more to do
> > > > > with the available manpower than anything else.
> > > >
> > > > Sure, but again, for subsystems, you have to define your threat model as
> > > > the LLMs are churning against the code base and coming up with lots of
> > > > crazy ideas if a device should or should not be trusted and spitting out
> > > > patches and reports like the ones that are in the first few patches of
> > > > this series.
> > > >
> > > > So please, pick a model, let's document it, and go with that. I am
> > > > getting directly conflicting responses here.
> > > >
> > > > thanks,
> > > >
> > > > greg k-h
> > >
> > > Supposed to be this one:
> > > Documentation/security/snp-tdx-threat-model.rst
> > >
> > > what is missing?
> >
> > A policy decision that needs to be made. All that document does is
> > describe a bunch of different "threats" yet does not decide what to do
> > about them at all from what I can tell.
>
> That would be this section I think:
>
> The **Linux kernel CoCo VM security objectives** can be summarized as follows:
>
> it does, indeed, not go into detail about how to interact, safely,
> with untrusted entities. Does it really need to be spelled out?
Seems like it as I didn't figure it out at all :)
> > And that's just for one subset of the CoC world, right? Is that
> > something that all virtio drivers need/want to care about?
>
> What is missing, and what you seem to be asking for, is an opinionated
> stance on which drivers we care about in this world?
> True.
Yes.
> coco guys tried to annotate drivers at some point to do exactly that.
> this was rejected upstream from the position that this is not
> different from handling buggy hardware, and just to fix all drivers.
> so it's up to users, and I guess for virtio the answer is yes
> with some exceptions because we don't have a better answer right now.
Ok, so back to the original question here:
> > So I don't see a real answer to the "does Linux trust the host to give
> > you good data or not" question in that file, am I missing it?
> >
> > thanks,
> >
> > greg k-h
>
> This? Note the last sentence.
>
> The **Linux CoCo VM attack surface** is any interface exposed from a CoCo
> guest Linux kernel towards an untrusted host that is not covered by the
> CoCo technology SW/HW protection. This includes any possible
> side-channels, as well as transient execution side channels. Examples of
> explicit (not side-channel) interfaces include accesses to port I/O, MMIO
> and DMA interfaces, access to PCI configuration space, VMM-specific
> hypercalls (towards Host-side VMM), access to shared memory pages,
> interrupts allowed to be injected into the guest kernel by the host, as
> well as CoCo technology-specific hypercalls, if present. Additionally, the
> host in a CoCo system typically controls the process of creating a CoCo
> guest: it has a method to load into a guest the firmware and bootloader
> images, the kernel image together with the kernel command line. All of this
> data should also be considered untrusted until its integrity and
> authenticity is established via attestation.
Great, so you are saying that we need to fix any bug found where a host
could be sending "bad" data over the virtio path before, and after, the
driver is bound to the device. That's a solid answer, and let's let the
LLMs run with that!
Which also implies that the first 3 patches here are acceptable, right?
:)
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v2] virtio-pmem: allocate flush bio from a driver-private bio_set
From: Pankaj Gupta @ 2026-07-17 15:03 UTC (permalink / raw)
To: Joseph Qi
Cc: Christoph Hellwig, Baokun Li, virtualization, linux-kernel,
Dan Williams, Michael S . Tsirkin, Alison Schofield, Linux NVDIMM,
Dave Jiang
In-Reply-To: <20260709124455.1547912-1-joseph.qi@linux.alibaba.com>
+CC more folks
> async_pmem_flush() allocates a child bio for the flush with GFP_ATOMIC.
> This runs from pmem_submit_bio(), a ->submit_bio callback that executes
> in a sleepable context, so there is no atomicity requirement here.
>
> bio_alloc() only guarantees success when __GFP_DIRECT_RECLAIM is set,
> because that is what lets it fall back to the mempool reserve. With
> GFP_ATOMIC the reclaim bit is absent, so the allocation can fail and
> return -ENOMEM whenever the fast paths (percpu cache and slab) are
> exhausted, which is common right after boot. A flush is issued from
> filesystem writeback and must not fail on a transient allocation
> shortage, otherwise the device can appear unmountable:
>
> Buffer I/O error on dev pmem0, logical block 0, lost sync page write
>
> Switch to GFP_NOIO so __GFP_DIRECT_RECLAIM is set and the allocation can
> make forward progress. However, bio_alloc() draws from the shared
> fs_bio_set, and the incoming bio being flushed may itself have come from
> fs_bio_set; allocating a second bio from the same set while submitting
> underneath ->submit_bio can deadlock the mempool. Add a driver-private
> bio_set for the flush and allocate from it via bio_alloc_bioset(), so
> the flush bio has an independent reserve.
>
> With a dedicated mempool-backed bio_set and GFP_NOIO the allocation
> cannot fail, so drop the now-redundant NULL check.
>
> Fixes: 6e84200c0a29 ("virtio-pmem: Add virtio pmem driver")
> Suggested-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> ---
> drivers/nvdimm/nd_virtio.c | 11 ++++++-----
> drivers/nvdimm/virtio_pmem.c | 11 ++++++++++-
> drivers/nvdimm/virtio_pmem.h | 4 ++++
> 3 files changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/nvdimm/nd_virtio.c b/drivers/nvdimm/nd_virtio.c
> index 4176046627beb..b4bd21edf5c1c 100644
> --- a/drivers/nvdimm/nd_virtio.c
> +++ b/drivers/nvdimm/nd_virtio.c
> @@ -110,17 +110,18 @@ static int virtio_pmem_flush(struct nd_region *nd_region)
> /* The asynchronous flush callback function */
> int async_pmem_flush(struct nd_region *nd_region, struct bio *bio)
> {
> + struct virtio_device *vdev = nd_region->provider_data;
> + struct virtio_pmem *vpmem = vdev->priv;
> +
> /*
> * Create child bio for asynchronous flush and chain with
> * parent bio. Otherwise directly call nd_region flush.
> */
> if (bio && bio->bi_iter.bi_sector != -1) {
> - struct bio *child = bio_alloc(bio->bi_bdev, 0,
> - REQ_OP_WRITE | REQ_PREFLUSH,
> - GFP_ATOMIC);
> + struct bio *child = bio_alloc_bioset(bio->bi_bdev, 0,
> + REQ_OP_WRITE | REQ_PREFLUSH, GFP_NOIO,
> + &vpmem->flush_bio_set);
>
> - if (!child)
> - return -ENOMEM;
> bio_clone_blkg_association(child, bio);
> child->bi_iter.bi_sector = -1;
> bio_chain(child, bio);
> diff --git a/drivers/nvdimm/virtio_pmem.c b/drivers/nvdimm/virtio_pmem.c
> index 77b1966619059..136179506b478 100644
> --- a/drivers/nvdimm/virtio_pmem.c
> +++ b/drivers/nvdimm/virtio_pmem.c
> @@ -65,12 +65,17 @@ static int virtio_pmem_probe(struct virtio_device *vdev)
> }
>
> mutex_init(&vpmem->flush_lock);
> + err = bioset_init(&vpmem->flush_bio_set, BIO_POOL_SIZE, 0, 0);
> + if (err) {
> + dev_err(&vdev->dev, "failed to initialize flush bio_set\n");
> + goto out_err;
> + }
> vpmem->vdev = vdev;
> vdev->priv = vpmem;
> err = init_vq(vpmem);
> if (err) {
> dev_err(&vdev->dev, "failed to initialize virtio pmem vq's\n");
> - goto out_err;
> + goto out_bioset;
> }
>
> if (virtio_has_feature(vdev, VIRTIO_PMEM_F_SHMEM_REGION)) {
> @@ -131,6 +136,8 @@ static int virtio_pmem_probe(struct virtio_device *vdev)
> nvdimm_bus_unregister(vpmem->nvdimm_bus);
> out_vq:
> vdev->config->del_vqs(vdev);
> +out_bioset:
> + bioset_exit(&vpmem->flush_bio_set);
> out_err:
> return err;
> }
> @@ -138,10 +145,12 @@ static int virtio_pmem_probe(struct virtio_device *vdev)
> static void virtio_pmem_remove(struct virtio_device *vdev)
> {
> struct nvdimm_bus *nvdimm_bus = dev_get_drvdata(&vdev->dev);
> + struct virtio_pmem *vpmem = vdev->priv;
>
> nvdimm_bus_unregister(nvdimm_bus);
> vdev->config->del_vqs(vdev);
> virtio_reset_device(vdev);
> + bioset_exit(&vpmem->flush_bio_set);
> }
>
> static int virtio_pmem_freeze(struct virtio_device *vdev)
> diff --git a/drivers/nvdimm/virtio_pmem.h b/drivers/nvdimm/virtio_pmem.h
> index f72cf17f9518f..4ff2076f75047 100644
> --- a/drivers/nvdimm/virtio_pmem.h
> +++ b/drivers/nvdimm/virtio_pmem.h
> @@ -15,6 +15,7 @@
> #include <linux/libnvdimm.h>
> #include <linux/mutex.h>
> #include <linux/spinlock.h>
> +#include <linux/bio.h>
>
> struct virtio_pmem_request {
> struct virtio_pmem_req req;
> @@ -39,6 +40,9 @@ struct virtio_pmem {
> /* Serialize flush requests to the device. */
> struct mutex flush_lock;
>
> + /* bio_set for allocating flush child bios */
> + struct bio_set flush_bio_set;
> +
> /* nvdimm bus registers virtio pmem device */
> struct nvdimm_bus *nvdimm_bus;
> struct nvdimm_bus_descriptor nd_desc;
> --
> 2.39.3
>
^ permalink raw reply
* [PATCH] virtio_console: take a kref in find_port_by_vq() to fix port UAF
From: Hari Mishal @ 2026-07-17 15:06 UTC (permalink / raw)
To: Amit Shah
Cc: Arnd Bergmann, Greg Kroah-Hartman, virtualization, linux-kernel,
Hari Mishal
find_port_by_vq() returns a raw struct port pointer without taking a
reference on it, unlike find_port_by_devt_in_portdev() which does.
find_port_by_vq()'s only two callers, in_intr() and out_intr(), run as
virtqueue interrupt callbacks, entirely independent of and possibly
concurrently with unplug_port(), which itself runs from a workqueue when
the host sends a VIRTIO_CONSOLE_PORT_REMOVE control message.
unplug_port() removes the port from portdev->ports under ports_lock,
then later drops its last reference with kref_put(), freeing it via
remove_port(). find_port_by_vq() also walks portdev->ports under
ports_lock, so if it finds the port still on the list, the list removal,
and therefore the eventual kref_put(), has not happened yet, and taking
a reference at that point is always safe. Without doing so,
in_intr()/out_intr() can be left holding a pointer to a port that
unplug_port() frees on another core before they are done using it.
Both triggers are host-controlled as the host decides when to send the
PORT_REMOVE control message and when to kick the port's data vq. So a
malicious backend could race the two on purpose, without any guest side
cooperation. The freed object is a generic kmalloc allocation containing
a wait_queue_head_t, which in_intr()/out_intr() pass to
wake_up_interruptible() after touching the stale pointer.
wake_up_interruptible() invokes a function pointer read out of the wait
queue's entries. If the freed slab slot is reclaimed with attacker
influenced content before that call, then this is an arbitrary function
call primitive rather than just undefined behaviour.
Take a reference in find_port_by_vq() while still holding ports_lock,
matching find_port_by_devt_in_portdev(), and release it in in_intr() and
out_intr() once they are done with the port.
Signed-off-by: Hari Mishal <harimishal1@gmail.com>
---
drivers/char/virtio_console.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index faef362dae85..1b63afe24e29 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -304,6 +304,12 @@ static struct port *find_port_by_id(struct ports_device *portdev, u32 id)
return port;
}
+/*
+ * Finds a port by the virtqueue and returns a pointer to struct port
+ * with the reference count incremented.
+ *
+ * Callers MUST decrement it when finished.
+ */
static struct port *find_port_by_vq(struct ports_device *portdev,
struct virtqueue *vq)
{
@@ -312,8 +318,10 @@ static struct port *find_port_by_vq(struct ports_device *portdev,
spin_lock_irqsave(&portdev->ports_lock, flags);
list_for_each_entry(port, &portdev->ports, list)
- if (port->in_vq == vq || port->out_vq == vq)
+ if (port->in_vq == vq || port->out_vq == vq) {
+ kref_get(&port->kref);
goto out;
+ }
port = NULL;
out:
spin_unlock_irqrestore(&portdev->ports_lock, flags);
@@ -1706,6 +1714,7 @@ static void out_intr(struct virtqueue *vq)
}
wake_up_interruptible(&port->waitqueue);
+ kref_put(&port->kref, remove_port);
}
static void in_intr(struct virtqueue *vq)
@@ -1723,6 +1732,7 @@ static void in_intr(struct virtqueue *vq)
if (!port->portdev) {
/* Port is being unplugged, ignore further data. */
spin_unlock_irqrestore(&port->inbuf_lock, flags);
+ kref_put(&port->kref, remove_port);
return;
}
port->inbuf = get_inbuf(port);
@@ -1756,6 +1766,8 @@ static void in_intr(struct virtqueue *vq)
if (is_console_port(port) && hvc_poll(port->cons.hvc))
hvc_kick();
+
+ kref_put(&port->kref, remove_port);
}
static void control_intr(struct virtqueue *vq)
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] vhost-scsi: Prevent OOM from invalid protection SGL count
From: Mike Christie @ 2026-07-17 15:13 UTC (permalink / raw)
To: Jia Jia, mst, jasowang, Linfeng Sun
Cc: pbonzini, stefanha, eperezma, virtualization, kvm, netdev,
linux-kernel, stable
In-Reply-To: <20260717142205.103515-1-physicalmtea@gmail.com>
cc'ing Linfeng.
Linfeng also reported this bug and provided a patch that has your check
and an extra check to avoid calling into the function in the first
place. It's been stuck in some other list.
I think it might be best for Linfeng to re-submit their patch now so it
can get merged. For that patch, I think there was one outstanding
question left where Michael had asked if the issue was found with AI so
the proper tag could be added. I don't think that question was responded to.
On 7/17/26 9:22 AM, Jia Jia wrote:
> The protection SGL path passes the result of vhost_scsi_calc_sgls()
> directly to sg_alloc_table_chained(). The helper returns a negative
> errno when the iterator is invalid or the request exceeds the segment
> limit. The negative errno is then treated as a very large unsigned count
> and sends the request into the SGL allocation path with an invalid size.
>
> Repeated malformed T10-PI submissions from a host-side application caused
> memory usage to rise sharply. MemAvailable fell to about 200 MB, and PSI
> full avg10 reached about 1.46. The OOM killer terminated several userspace
> processes before the endpoint cleanup completed. The kernel log included:
>
> [17036.451028] Out of memory: Killed process 2345 (systemd)
> [17036.493325] Out of memory: Killed process 2349 (sd-pam)
> [17078.265127] Out of memory: Killed process 1793 (networkd-dispat)
>
> Return the calculation error before setting up the protection SGL. This
> keeps the protection path consistent with the data SGL path and prevents
> the invalid count from entering the allocation path.
>
> Fixes: bca939d5bcd0 ("vhost-scsi: Dynamically allocate scatterlists")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jia Jia <physicalmtea@gmail.com>
> ---
> drivers/vhost/scsi.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index 9a1253b9d..8486652fd 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -972,6 +972,9 @@ vhost_scsi_mapal(struct vhost_scsi *vs, struct vhost_scsi_cmd *cmd,
> if (prot_bytes) {
> sgl_count = vhost_scsi_calc_sgls(prot_iter, prot_bytes,
> VHOST_SCSI_PREALLOC_PROT_SGLS);
> + if (sgl_count < 0)
> + return sgl_count;
> +
> cmd->prot_table.sgl = cmd->prot_sgl;
> ret = sg_alloc_table_chained(&cmd->prot_table, sgl_count,
> cmd->prot_table.sgl,
> --
> 2.43.0
>
^ permalink raw reply
* RE: [PATCH v2 1/4] virtio-mem: validate device-reported block size
From: Michael Kelley @ 2026-07-17 15:27 UTC (permalink / raw)
To: Greg Kroah-Hartman, Michael S. Tsirkin
Cc: David Hildenbrand (Arm), Hari Mishal, Jason Wang, Xuan Zhuo,
Eugenio Pérez, virtualization@lists.linux.dev,
linux-kernel@vger.kernel.org, elena.reshetova@intel.com,
carlos.bilbao.osdev@gmail.com
In-Reply-To: <2026071724-modulator-pliable-fa1c@gregkh>
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Sent: Friday, July 17, 2026 7:31 AM
> On Fri, Jul 17, 2026 at 09:08:23AM -0400, Michael S. Tsirkin wrote:
> > On Fri, Jul 17, 2026 at 02:07:50PM +0200, Greg Kroah-Hartman wrote:
> > > On Fri, Jul 17, 2026 at 06:52:46AM -0400, Michael S. Tsirkin wrote:
> > > > On Fri, Jul 17, 2026 at 12:46:52PM +0200, Greg Kroah-Hartman wrote:
> > > > > On Fri, Jul 17, 2026 at 06:23:57AM -0400, Michael S. Tsirkin wrote:
> > > > > > On Fri, Jul 17, 2026 at 12:15:09PM +0200, Greg Kroah-Hartman wrote:
> > > > > > > On Fri, Jul 17, 2026 at 06:10:41AM -0400, Michael S. Tsirkin wrote:
> > > > > > > > On Fri, Jul 17, 2026 at 11:14:23AM +0200, Greg Kroah-Hartman wrote:
> > > > > > > > > On Fri, Jul 17, 2026 at 04:59:32AM -0400, Michael S. Tsirkin wrote:
> > > > > > > > > > On Fri, Jul 17, 2026 at 10:39:40AM +0200, David Hildenbrand (Arm) wrote:
> > > > > > > > > > > On 7/17/26 07:48, Michael S. Tsirkin wrote:
> > > > > > > > > > > > On Thu, Jul 16, 2026 at 05:59:05PM +0200, David Hildenbrand (Arm) wrote:
> > > > > > > > > > > >>> Or do we just always trust virtio mem devices explicitly?
> > > > > > > > > > > >>
> > > > > > > > > > > >> It's hard for me to understand where we draw the line, really.
> > > > > > > > > > > >>
> > > > > > > > > > > >> But maybe MST can clarify what we care about in virtio world where the
> > > > > > > > > > > >> hypervisor is fully in charge of the device,
> > > > > > > > > > > >
> > > > > > > > > > > > Generally:
> > > > > > > > > > > > - The guest is expected to whitelist drivers (most drivers have not
> > > > > > > > > > > > been audited).
> > > > > > > > > > >
> > > > > > > > > > > But even if you audited your driver, who makes sure that we consider all ways
> > > > > > > > > > > where the device could mess with us?
> > > > > > > > > >
> > > > > > > > > > A lot of this is up to a correct setup. For example, make sure all
> > > > > > > > > > filesystems are encrypted and refuse to mount unencrypted ones.
> > > > > > > > > >
> > > > > > > > > > > Something feels off here.
> > > > > > > > > > >
> > > > > > > > > > > Handling selected out-of-spec scenarios like this feels like a band-aid. Happy
> > > > > > > > > > > to be corrected.
> > > > > > > > > >
> > > > > > > > > > Well Documentation/security/snp-tdx-threat-model.rst puts it like this:
> > > > > > > > > > It is important to note
> > > > > > > > > > that this doesn’t imply that the host or VMM are intentionally
> > > > > > > > > > malicious, but that there exists a security value in having a small CoCo
> > > > > > > > > > VM TCB.
> > > > > > > > > >
> > > > > > > > > > and
> > > > > > > > > >
> > > > > > > > > > While traditionally the host has unlimited access to guest data and can
> > > > > > > > > > leverage this access to attack the guest, the CoCo systems mitigate such
> > > > > > > > > > attacks by adding security features like guest data confidentiality and
> > > > > > > > > > integrity protection.
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > now, when we are talking about "mitigation" it is indeed becoming a bit
> > > > > > > > > > murky.
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > For me, a rule of thumb I came up with is that if the validation happens
> > > > > > > > > > to also be helful for users e.g. to work around buggy devices,
> > > > > > > > > > or maybe because we feel failing gracefully is nice because this
> > > > > > > > > > will allow to later make use of this config and old drivers will
> > > > > > > > > > fail but at least not panic, then it is good to include.
> > > > > > > > >
> > > > > > > > > Why not do what USB does? Don't trust the device until AFTER probe()
> > > > > > > > > succeeds? All of the needed checking should happen before then, as that
> > > > > > > > > is a "slow path" so lots of validation and the like can happen at that
> > > > > > > > > point.
> > > > > > > > >
> > > > > > > > > After that, during the normal data paths, after the driver is bound,
> > > > > > > > > trust it all you want as attempting to validate every single packet is
> > > > > > > > > just going to be impossible.
> > > > > > > > >
> > > > > > > > > thanks,
> > > > > > > > >
> > > > > > > > > greg k-h
> > > > > > > >
> > > > > > > > People do expect that data path validation at this point.
> > > > > > >
> > > > > > > Ok, so you want this patch :)
> > > > > > >
> > > > > > > And more, as you need to treat everything from the host as "untrusted",
> > > > > > > and it must be "verified".
> > > > > >
> > > > > > Well. First it's not me) Second it's only specific configurations -
> > > > > > for example there's no short term plan to validate filesystem code, people
> > > > > > are expected to rely on encryption. The reasons have more to do
> > > > > > with the available manpower than anything else.
> > > > >
> > > > > Sure, but again, for subsystems, you have to define your threat model as
> > > > > the LLMs are churning against the code base and coming up with lots of
> > > > > crazy ideas if a device should or should not be trusted and spitting out
> > > > > patches and reports like the ones that are in the first few patches of
> > > > > this series.
> > > > >
> > > > > So please, pick a model, let's document it, and go with that. I am
> > > > > getting directly conflicting responses here.
> > > > >
> > > > > thanks,
> > > > >
> > > > > greg k-h
> > > >
> > > > Supposed to be this one:
> > > > Documentation/security/snp-tdx-threat-model.rst
> > > >
> > > > what is missing?
> > >
> > > A policy decision that needs to be made. All that document does is
> > > describe a bunch of different "threats" yet does not decide what to do
> > > about them at all from what I can tell.
> >
> > That would be this section I think:
> >
> > The **Linux kernel CoCo VM security objectives** can be summarized as follows:
> >
> > it does, indeed, not go into detail about how to interact, safely,
> > with untrusted entities. Does it really need to be spelled out?
>
> Seems like it as I didn't figure it out at all :)
>
> > > And that's just for one subset of the CoC world, right? Is that
> > > something that all virtio drivers need/want to care about?
> >
> > What is missing, and what you seem to be asking for, is an opinionated
> > stance on which drivers we care about in this world?
> > True.
>
> Yes.
>
> > coco guys tried to annotate drivers at some point to do exactly that.
> > this was rejected upstream from the position that this is not
> > different from handling buggy hardware, and just to fix all drivers.
> > so it's up to users, and I guess for virtio the answer is yes
> > with some exceptions because we don't have a better answer right now.
>
> Ok, so back to the original question here:
>
> > > So I don't see a real answer to the "does Linux trust the host to give
> > > you good data or not" question in that file, am I missing it?
> > >
> > > thanks,
> > >
> > > greg k-h
> >
> > This? Note the last sentence.
> >
> > The **Linux CoCo VM attack surface** is any interface exposed from a CoCo
> > guest Linux kernel towards an untrusted host that is not covered by the
> > CoCo technology SW/HW protection. This includes any possible
> > side-channels, as well as transient execution side channels. Examples of
> > explicit (not side-channel) interfaces include accesses to port I/O, MMIO
> > and DMA interfaces, access to PCI configuration space, VMM-specific
> > hypercalls (towards Host-side VMM), access to shared memory pages,
> > interrupts allowed to be injected into the guest kernel by the host, as
> > well as CoCo technology-specific hypercalls, if present. Additionally, the
> > host in a CoCo system typically controls the process of creating a CoCo
> > guest: it has a method to load into a guest the firmware and bootloader
> > images, the kernel image together with the kernel command line. All of this
> > data should also be considered untrusted until its integrity and
> > authenticity is established via attestation.
>
> Great, so you are saying that we need to fix any bug found where a host
> could be sending "bad" data over the virtio path before, and after, the
> driver is bound to the device. That's a solid answer, and let's let the
> LLMs run with that!
>
FWIW, Hyper-V VMBus drivers in Linux use this same threat model,
with the same conclusions about data validation. The details are in
Documentation/virt/hyperv/coco.rst. See the Section entitled
" Guest communication with Hyper-V". Let me know if you've come
across potential data leaks or corruption modalities in the virtio code
that might also be applicable to the VMBus drivers, that aren't already
mentioned in that documentation.
Michael
^ permalink raw reply
* Re: [PATCH v2 1/4] virtio-mem: validate device-reported block size
From: Michael S. Tsirkin @ 2026-07-17 16:28 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: David Hildenbrand (Arm), Hari Mishal, Jason Wang, Xuan Zhuo,
Eugenio Pérez, virtualization, linux-kernel, elena.reshetova,
carlos.bilbao.osdev
In-Reply-To: <2026071724-modulator-pliable-fa1c@gregkh>
On Fri, Jul 17, 2026 at 04:31:03PM +0200, Greg Kroah-Hartman wrote:
> On Fri, Jul 17, 2026 at 09:08:23AM -0400, Michael S. Tsirkin wrote:
> > On Fri, Jul 17, 2026 at 02:07:50PM +0200, Greg Kroah-Hartman wrote:
> > > On Fri, Jul 17, 2026 at 06:52:46AM -0400, Michael S. Tsirkin wrote:
> > > > On Fri, Jul 17, 2026 at 12:46:52PM +0200, Greg Kroah-Hartman wrote:
> > > > > On Fri, Jul 17, 2026 at 06:23:57AM -0400, Michael S. Tsirkin wrote:
> > > > > > On Fri, Jul 17, 2026 at 12:15:09PM +0200, Greg Kroah-Hartman wrote:
> > > > > > > On Fri, Jul 17, 2026 at 06:10:41AM -0400, Michael S. Tsirkin wrote:
> > > > > > > > On Fri, Jul 17, 2026 at 11:14:23AM +0200, Greg Kroah-Hartman wrote:
> > > > > > > > > On Fri, Jul 17, 2026 at 04:59:32AM -0400, Michael S. Tsirkin wrote:
> > > > > > > > > > On Fri, Jul 17, 2026 at 10:39:40AM +0200, David Hildenbrand (Arm) wrote:
> > > > > > > > > > > On 7/17/26 07:48, Michael S. Tsirkin wrote:
> > > > > > > > > > > > On Thu, Jul 16, 2026 at 05:59:05PM +0200, David Hildenbrand (Arm) wrote:
> > > > > > > > > > > >>> Or do we just always trust virtio mem devices explicitly?
> > > > > > > > > > > >>
> > > > > > > > > > > >> It's hard for me to understand where we draw the line, really.
> > > > > > > > > > > >>
> > > > > > > > > > > >> But maybe MST can clarify what we care about in virtio world where the
> > > > > > > > > > > >> hypervisor is fully in charge of the device,
> > > > > > > > > > > >
> > > > > > > > > > > > Generally:
> > > > > > > > > > > > - The guest is expected to whitelist drivers (most drivers have not
> > > > > > > > > > > > been audited).
> > > > > > > > > > >
> > > > > > > > > > > But even if you audited your driver, who makes sure that we consider all ways
> > > > > > > > > > > where the device could mess with us?
> > > > > > > > > >
> > > > > > > > > > A lot of this is up to a correct setup. For example, make sure all
> > > > > > > > > > filesystems are encrypted and refuse to mount unencrypted ones.
> > > > > > > > > >
> > > > > > > > > > > Something feels off here.
> > > > > > > > > > >
> > > > > > > > > > > Handling selected out-of-spec scenarios like this feels like a band-aid. Happy
> > > > > > > > > > > to be corrected.
> > > > > > > > > >
> > > > > > > > > > Well Documentation/security/snp-tdx-threat-model.rst puts it like this:
> > > > > > > > > > It is important to note
> > > > > > > > > > that this doesn’t imply that the host or VMM are intentionally
> > > > > > > > > > malicious, but that there exists a security value in having a small CoCo
> > > > > > > > > > VM TCB.
> > > > > > > > > >
> > > > > > > > > > and
> > > > > > > > > >
> > > > > > > > > > While traditionally the host has unlimited access to guest data and can
> > > > > > > > > > leverage this access to attack the guest, the CoCo systems mitigate such
> > > > > > > > > > attacks by adding security features like guest data confidentiality and
> > > > > > > > > > integrity protection.
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > now, when we are talking about "mitigation" it is indeed becoming a bit
> > > > > > > > > > murky.
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > For me, a rule of thumb I came up with is that if the validation happens
> > > > > > > > > > to also be helful for users e.g. to work around buggy devices,
> > > > > > > > > > or maybe because we feel failing gracefully is nice because this
> > > > > > > > > > will allow to later make use of this config and old drivers will
> > > > > > > > > > fail but at least not panic, then it is good to include.
> > > > > > > > >
> > > > > > > > > Why not do what USB does? Don't trust the device until AFTER probe()
> > > > > > > > > succeeds? All of the needed checking should happen before then, as that
> > > > > > > > > is a "slow path" so lots of validation and the like can happen at that
> > > > > > > > > point.
> > > > > > > > >
> > > > > > > > > After that, during the normal data paths, after the driver is bound,
> > > > > > > > > trust it all you want as attempting to validate every single packet is
> > > > > > > > > just going to be impossible.
> > > > > > > > >
> > > > > > > > > thanks,
> > > > > > > > >
> > > > > > > > > greg k-h
> > > > > > > >
> > > > > > > > People do expect that data path validation at this point.
> > > > > > >
> > > > > > > Ok, so you want this patch :)
> > > > > > >
> > > > > > > And more, as you need to treat everything from the host as "untrusted",
> > > > > > > and it must be "verified".
> > > > > >
> > > > > > Well. First it's not me) Second it's only specific configurations -
> > > > > > for example there's no short term plan to validate filesystem code, people
> > > > > > are expected to rely on encryption. The reasons have more to do
> > > > > > with the available manpower than anything else.
> > > > >
> > > > > Sure, but again, for subsystems, you have to define your threat model as
> > > > > the LLMs are churning against the code base and coming up with lots of
> > > > > crazy ideas if a device should or should not be trusted and spitting out
> > > > > patches and reports like the ones that are in the first few patches of
> > > > > this series.
> > > > >
> > > > > So please, pick a model, let's document it, and go with that. I am
> > > > > getting directly conflicting responses here.
> > > > >
> > > > > thanks,
> > > > >
> > > > > greg k-h
> > > >
> > > > Supposed to be this one:
> > > > Documentation/security/snp-tdx-threat-model.rst
> > > >
> > > > what is missing?
> > >
> > > A policy decision that needs to be made. All that document does is
> > > describe a bunch of different "threats" yet does not decide what to do
> > > about them at all from what I can tell.
> >
> > That would be this section I think:
> >
> > The **Linux kernel CoCo VM security objectives** can be summarized as follows:
> >
> > it does, indeed, not go into detail about how to interact, safely,
> > with untrusted entities. Does it really need to be spelled out?
>
> Seems like it as I didn't figure it out at all :)
>
> > > And that's just for one subset of the CoC world, right? Is that
> > > something that all virtio drivers need/want to care about?
> >
> > What is missing, and what you seem to be asking for, is an opinionated
> > stance on which drivers we care about in this world?
> > True.
>
> Yes.
>
> > coco guys tried to annotate drivers at some point to do exactly that.
> > this was rejected upstream from the position that this is not
> > different from handling buggy hardware, and just to fix all drivers.
> > so it's up to users, and I guess for virtio the answer is yes
> > with some exceptions because we don't have a better answer right now.
>
> Ok, so back to the original question here:
>
> > > So I don't see a real answer to the "does Linux trust the host to give
> > > you good data or not" question in that file, am I missing it?
> > >
> > > thanks,
> > >
> > > greg k-h
> >
> > This? Note the last sentence.
> >
> > The **Linux CoCo VM attack surface** is any interface exposed from a CoCo
> > guest Linux kernel towards an untrusted host that is not covered by the
> > CoCo technology SW/HW protection. This includes any possible
> > side-channels, as well as transient execution side channels. Examples of
> > explicit (not side-channel) interfaces include accesses to port I/O, MMIO
> > and DMA interfaces, access to PCI configuration space, VMM-specific
> > hypercalls (towards Host-side VMM), access to shared memory pages,
> > interrupts allowed to be injected into the guest kernel by the host, as
> > well as CoCo technology-specific hypercalls, if present. Additionally, the
> > host in a CoCo system typically controls the process of creating a CoCo
> > guest: it has a method to load into a guest the firmware and bootloader
> > images, the kernel image together with the kernel command line. All of this
> > data should also be considered untrusted until its integrity and
> > authenticity is established via attestation.
>
> Great, so you are saying that we need to fix any bug found where a host
> could be sending "bad" data over the virtio path before, and after, the
> driver is bound to the device. That's a solid answer, and let's let the
> LLMs run with that!
>
> Which also implies that the first 3 patches here are acceptable, right?
> :)
>
> thanks,
>
> greg k-h
The 1st patch has nothing to do with coco. Bad values it checks do not
lead to any information leak. I'm fine with it as such but I
would like the commit log to make that clearer.
--
MST
^ permalink raw reply
* Re: [PATCH v2 1/4] virtio-mem: validate device-reported block size
From: Michael S. Tsirkin @ 2026-07-17 16:30 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: David Hildenbrand (Arm), Hari Mishal, Jason Wang, Xuan Zhuo,
Eugenio Pérez, virtualization, linux-kernel, elena.reshetova,
carlos.bilbao.osdev
In-Reply-To: <20260717122729-mutt-send-email-mst@kernel.org>
On Fri, Jul 17, 2026 at 12:28:45PM -0400, Michael S. Tsirkin wrote:
> On Fri, Jul 17, 2026 at 04:31:03PM +0200, Greg Kroah-Hartman wrote:
> > On Fri, Jul 17, 2026 at 09:08:23AM -0400, Michael S. Tsirkin wrote:
> > > On Fri, Jul 17, 2026 at 02:07:50PM +0200, Greg Kroah-Hartman wrote:
> > > > On Fri, Jul 17, 2026 at 06:52:46AM -0400, Michael S. Tsirkin wrote:
> > > > > On Fri, Jul 17, 2026 at 12:46:52PM +0200, Greg Kroah-Hartman wrote:
> > > > > > On Fri, Jul 17, 2026 at 06:23:57AM -0400, Michael S. Tsirkin wrote:
> > > > > > > On Fri, Jul 17, 2026 at 12:15:09PM +0200, Greg Kroah-Hartman wrote:
> > > > > > > > On Fri, Jul 17, 2026 at 06:10:41AM -0400, Michael S. Tsirkin wrote:
> > > > > > > > > On Fri, Jul 17, 2026 at 11:14:23AM +0200, Greg Kroah-Hartman wrote:
> > > > > > > > > > On Fri, Jul 17, 2026 at 04:59:32AM -0400, Michael S. Tsirkin wrote:
> > > > > > > > > > > On Fri, Jul 17, 2026 at 10:39:40AM +0200, David Hildenbrand (Arm) wrote:
> > > > > > > > > > > > On 7/17/26 07:48, Michael S. Tsirkin wrote:
> > > > > > > > > > > > > On Thu, Jul 16, 2026 at 05:59:05PM +0200, David Hildenbrand (Arm) wrote:
> > > > > > > > > > > > >>> Or do we just always trust virtio mem devices explicitly?
> > > > > > > > > > > > >>
> > > > > > > > > > > > >> It's hard for me to understand where we draw the line, really.
> > > > > > > > > > > > >>
> > > > > > > > > > > > >> But maybe MST can clarify what we care about in virtio world where the
> > > > > > > > > > > > >> hypervisor is fully in charge of the device,
> > > > > > > > > > > > >
> > > > > > > > > > > > > Generally:
> > > > > > > > > > > > > - The guest is expected to whitelist drivers (most drivers have not
> > > > > > > > > > > > > been audited).
> > > > > > > > > > > >
> > > > > > > > > > > > But even if you audited your driver, who makes sure that we consider all ways
> > > > > > > > > > > > where the device could mess with us?
> > > > > > > > > > >
> > > > > > > > > > > A lot of this is up to a correct setup. For example, make sure all
> > > > > > > > > > > filesystems are encrypted and refuse to mount unencrypted ones.
> > > > > > > > > > >
> > > > > > > > > > > > Something feels off here.
> > > > > > > > > > > >
> > > > > > > > > > > > Handling selected out-of-spec scenarios like this feels like a band-aid. Happy
> > > > > > > > > > > > to be corrected.
> > > > > > > > > > >
> > > > > > > > > > > Well Documentation/security/snp-tdx-threat-model.rst puts it like this:
> > > > > > > > > > > It is important to note
> > > > > > > > > > > that this doesn’t imply that the host or VMM are intentionally
> > > > > > > > > > > malicious, but that there exists a security value in having a small CoCo
> > > > > > > > > > > VM TCB.
> > > > > > > > > > >
> > > > > > > > > > > and
> > > > > > > > > > >
> > > > > > > > > > > While traditionally the host has unlimited access to guest data and can
> > > > > > > > > > > leverage this access to attack the guest, the CoCo systems mitigate such
> > > > > > > > > > > attacks by adding security features like guest data confidentiality and
> > > > > > > > > > > integrity protection.
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > now, when we are talking about "mitigation" it is indeed becoming a bit
> > > > > > > > > > > murky.
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > For me, a rule of thumb I came up with is that if the validation happens
> > > > > > > > > > > to also be helful for users e.g. to work around buggy devices,
> > > > > > > > > > > or maybe because we feel failing gracefully is nice because this
> > > > > > > > > > > will allow to later make use of this config and old drivers will
> > > > > > > > > > > fail but at least not panic, then it is good to include.
> > > > > > > > > >
> > > > > > > > > > Why not do what USB does? Don't trust the device until AFTER probe()
> > > > > > > > > > succeeds? All of the needed checking should happen before then, as that
> > > > > > > > > > is a "slow path" so lots of validation and the like can happen at that
> > > > > > > > > > point.
> > > > > > > > > >
> > > > > > > > > > After that, during the normal data paths, after the driver is bound,
> > > > > > > > > > trust it all you want as attempting to validate every single packet is
> > > > > > > > > > just going to be impossible.
> > > > > > > > > >
> > > > > > > > > > thanks,
> > > > > > > > > >
> > > > > > > > > > greg k-h
> > > > > > > > >
> > > > > > > > > People do expect that data path validation at this point.
> > > > > > > >
> > > > > > > > Ok, so you want this patch :)
> > > > > > > >
> > > > > > > > And more, as you need to treat everything from the host as "untrusted",
> > > > > > > > and it must be "verified".
> > > > > > >
> > > > > > > Well. First it's not me) Second it's only specific configurations -
> > > > > > > for example there's no short term plan to validate filesystem code, people
> > > > > > > are expected to rely on encryption. The reasons have more to do
> > > > > > > with the available manpower than anything else.
> > > > > >
> > > > > > Sure, but again, for subsystems, you have to define your threat model as
> > > > > > the LLMs are churning against the code base and coming up with lots of
> > > > > > crazy ideas if a device should or should not be trusted and spitting out
> > > > > > patches and reports like the ones that are in the first few patches of
> > > > > > this series.
> > > > > >
> > > > > > So please, pick a model, let's document it, and go with that. I am
> > > > > > getting directly conflicting responses here.
> > > > > >
> > > > > > thanks,
> > > > > >
> > > > > > greg k-h
> > > > >
> > > > > Supposed to be this one:
> > > > > Documentation/security/snp-tdx-threat-model.rst
> > > > >
> > > > > what is missing?
> > > >
> > > > A policy decision that needs to be made. All that document does is
> > > > describe a bunch of different "threats" yet does not decide what to do
> > > > about them at all from what I can tell.
> > >
> > > That would be this section I think:
> > >
> > > The **Linux kernel CoCo VM security objectives** can be summarized as follows:
> > >
> > > it does, indeed, not go into detail about how to interact, safely,
> > > with untrusted entities. Does it really need to be spelled out?
> >
> > Seems like it as I didn't figure it out at all :)
> >
> > > > And that's just for one subset of the CoC world, right? Is that
> > > > something that all virtio drivers need/want to care about?
> > >
> > > What is missing, and what you seem to be asking for, is an opinionated
> > > stance on which drivers we care about in this world?
> > > True.
> >
> > Yes.
> >
> > > coco guys tried to annotate drivers at some point to do exactly that.
> > > this was rejected upstream from the position that this is not
> > > different from handling buggy hardware, and just to fix all drivers.
> > > so it's up to users, and I guess for virtio the answer is yes
> > > with some exceptions because we don't have a better answer right now.
> >
> > Ok, so back to the original question here:
> >
> > > > So I don't see a real answer to the "does Linux trust the host to give
> > > > you good data or not" question in that file, am I missing it?
> > > >
> > > > thanks,
> > > >
> > > > greg k-h
> > >
> > > This? Note the last sentence.
> > >
> > > The **Linux CoCo VM attack surface** is any interface exposed from a CoCo
> > > guest Linux kernel towards an untrusted host that is not covered by the
> > > CoCo technology SW/HW protection. This includes any possible
> > > side-channels, as well as transient execution side channels. Examples of
> > > explicit (not side-channel) interfaces include accesses to port I/O, MMIO
> > > and DMA interfaces, access to PCI configuration space, VMM-specific
> > > hypercalls (towards Host-side VMM), access to shared memory pages,
> > > interrupts allowed to be injected into the guest kernel by the host, as
> > > well as CoCo technology-specific hypercalls, if present. Additionally, the
> > > host in a CoCo system typically controls the process of creating a CoCo
> > > guest: it has a method to load into a guest the firmware and bootloader
> > > images, the kernel image together with the kernel command line. All of this
> > > data should also be considered untrusted until its integrity and
> > > authenticity is established via attestation.
> >
> > Great, so you are saying that we need to fix any bug found where a host
> > could be sending "bad" data over the virtio path before, and after, the
> > driver is bound to the device. That's a solid answer, and let's let the
> > LLMs run with that!
> >
> > Which also implies that the first 3 patches here are acceptable, right?
> > :)
> >
> > thanks,
> >
> > greg k-h
>
> The 1st patch has nothing to do with coco. Bad values it checks do not
> lead to any information leak. I'm fine with it as such
> but I
> would like the commit log to make that clearer.
Or to be more precise, I'd like the motivation to be documented,
given it is not coco, and then I can judge the merits.
>
> --
> MST
^ permalink raw reply
* Re: [PATCH] vhost-scsi: Prevent OOM from invalid protection SGL count
From: Michael S. Tsirkin @ 2026-07-17 16:31 UTC (permalink / raw)
To: Mike Christie
Cc: Jia Jia, jasowang, Linfeng Sun, pbonzini, stefanha, eperezma,
virtualization, kvm, netdev, linux-kernel, stable
In-Reply-To: <8f5b5350-0be9-4b79-a6c9-069d418dee30@oracle.com>
On Fri, Jul 17, 2026 at 10:13:36AM -0500, Mike Christie wrote:
> cc'ing Linfeng.
>
> Linfeng also reported this bug and provided a patch that has your check and
> an extra check to avoid calling into the function in the first place. It's
> been stuck in some other list.
>
> I think it might be best for Linfeng to re-submit their patch now so it can
> get merged. For that patch, I think there was one outstanding question left
> where Michael had asked if the issue was found with AI so the proper tag
> could be added. I don't think that question was responded to.
Right, was waiting for that.
>
> On 7/17/26 9:22 AM, Jia Jia wrote:
> > The protection SGL path passes the result of vhost_scsi_calc_sgls()
> > directly to sg_alloc_table_chained(). The helper returns a negative
> > errno when the iterator is invalid or the request exceeds the segment
> > limit. The negative errno is then treated as a very large unsigned count
> > and sends the request into the SGL allocation path with an invalid size.
> >
> > Repeated malformed T10-PI submissions from a host-side application caused
> > memory usage to rise sharply. MemAvailable fell to about 200 MB, and PSI
> > full avg10 reached about 1.46. The OOM killer terminated several userspace
> > processes before the endpoint cleanup completed. The kernel log included:
> >
> > [17036.451028] Out of memory: Killed process 2345 (systemd)
> > [17036.493325] Out of memory: Killed process 2349 (sd-pam)
> > [17078.265127] Out of memory: Killed process 1793 (networkd-dispat)
> >
> > Return the calculation error before setting up the protection SGL. This
> > keeps the protection path consistent with the data SGL path and prevents
> > the invalid count from entering the allocation path.
> >
> > Fixes: bca939d5bcd0 ("vhost-scsi: Dynamically allocate scatterlists")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Jia Jia <physicalmtea@gmail.com>
> > ---
> > drivers/vhost/scsi.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> > index 9a1253b9d..8486652fd 100644
> > --- a/drivers/vhost/scsi.c
> > +++ b/drivers/vhost/scsi.c
> > @@ -972,6 +972,9 @@ vhost_scsi_mapal(struct vhost_scsi *vs, struct vhost_scsi_cmd *cmd,
> > if (prot_bytes) {
> > sgl_count = vhost_scsi_calc_sgls(prot_iter, prot_bytes,
> > VHOST_SCSI_PREALLOC_PROT_SGLS);
> > + if (sgl_count < 0)
> > + return sgl_count;
> > +
> > cmd->prot_table.sgl = cmd->prot_sgl;
> > ret = sg_alloc_table_chained(&cmd->prot_table, sgl_count,
> > cmd->prot_table.sgl,
> > --
> > 2.43.0
> >
^ permalink raw reply
* Re: [PATCH v4 2/8] media: virtio: Add virtio-media driver structs and function declarations
From: Brian Daniels @ 2026-07-17 19:46 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Mauro Carvalho Chehab, acourbot, adelva, aesteve, changyeon,
daniel.almeida, eperezma, gnurou, gurchetansingh, hverkuil,
jasowang, linux-kernel, linux-media, nicolas.dufresne,
virtualization, xuanzhuo
In-Reply-To: <20260622170708-mutt-send-email-mst@kernel.org>
On Mon, Jun 22, 2026 at 5:08 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Mon, Jun 22, 2026 at 04:43:37PM -0400, Brian Daniels wrote:
> > From: Alexandre Courbot <gnurou@gmail.com>
> >
> > Add the structs and function declarations for the new virtio-media drvier.
> >
> > Signed-off-by: Alexandre Courbot <gnurou@gmail.com>
> > Co-developed-by: Brian Daniels <briandaniels@google.com>
> > Signed-off-by: Brian Daniels <briandaniels@google.com>
> > ---
> > drivers/media/virtio/virtio_media.h | 95 +++++++++++++++++++++++++++++
> > 1 file changed, 95 insertions(+)
> > create mode 100644 drivers/media/virtio/virtio_media.h
> >
> > diff --git a/drivers/media/virtio/virtio_media.h b/drivers/media/virtio/virtio_media.h
> > new file mode 100644
> > index 000000000..52809d4e9
> > --- /dev/null
> > +++ b/drivers/media/virtio/virtio_media.h
> > @@ -0,0 +1,95 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0+ */
> > +
> > +/*
> > + * Virtio-media structures & functions declarations.
> > + *
> > + * Copyright (c) 2024-2025 Google LLC.
> > + */
> > +
> > +#ifndef __VIRTIO_MEDIA_H
> > +#define __VIRTIO_MEDIA_H
> > +
> > +#include <linux/virtio_config.h>
> > +#include <media/v4l2-device.h>
> > +
> > +#include "protocol.h"
> > +
> > +#define DESC_CHAIN_MAX_LEN SG_MAX_SINGLE_ALLOC
> > +
> > +#define VIRTIO_MEDIA_DEFAULT_DRIVER_NAME "virtio-media"
> > +
> > +extern char *virtio_media_driver_name;
> > +extern bool virtio_media_allow_userptr;
> > +
> > +/**
> > + * struct virtio_media - Virtio-media device.
> > + * @v4l2_dev: v4l2_device for the media device.
> > + * @video_dev: video_device for the media device.
> > + * @virtio_dev: virtio device for the media device.
> > + * @commandq: virtio command queue.
> > + * @eventq: virtio event queue.
> > + * @eventq_work: work to run when events are received on @eventq.
> > + * @mmap_region: region into which MMAP buffers are mapped by the host.
> > + * @event_buffer: buffer for event descriptors.
> > + * @sessions: list of active sessions on the device.
> > + * @sessions_lock: protects @sessions and ``virtio_media_session::list``.
> > + * @events_lock: prevents concurrent processing of events.
> > + * @cmd: union of the device commands "open" and "munmap". The other
> > + * commands are handled by @struct virtio_media_session
> > + * @resp: union of responses.to device commands "open" and "munmap". The
> > + * other responses are handled by @struct virtio_media_session
> > + * @vlock: serializes access to the command queue.
> > + * @wq: waitqueue for host responses on the command queue.
> > + */
> > +struct virtio_media {
> > + struct v4l2_device v4l2_dev;
> > + struct video_device video_dev;
> > +
> > + struct virtio_device *virtio_dev;
> > + struct virtqueue *commandq;
> > + struct virtqueue *eventq;
> > + struct work_struct eventq_work;
> > +
> > + struct virtio_shm_region mmap_region;
> > +
> > + void *event_buffer;
> > +
> > + struct list_head sessions;
> > + struct mutex sessions_lock; /* protects sessions list */
> > +
> > + struct mutex events_lock; /* prevents concurrent event processing */
> > +
> > + union {
> > + struct virtio_media_cmd_open open;
> > + struct virtio_media_cmd_munmap munmap;
> > + } cmd;
> > +
> > + union {
> > + struct virtio_media_resp_open open;
> > + struct virtio_media_resp_munmap munmap;
> > + } resp;
>
>
> You need DMA alignment padding for these things.
Each member of the above unions is already padded to 64-bit alignment,
is that sufficient?
If not, could you tell me what else is necessary?
> Which one can only see when I reads the actual driver 8 patches down.
> Which is why it's not a sensible way to split patches.
>
> A sensible way is to have a driver then add functionality
> in logical pieces gradually.
>
Appreciate the feedback. There was a previous comment requesting to
split the patch into c/h file pairs, but I can try reformatting it as
you suggest for v5.
> > +
> > + struct mutex vlock; /* serializes command queue access */
> > + wait_queue_head_t wq;
> > +};
> > +
> > +static inline struct virtio_media *
> > +to_virtio_media(struct video_device *video_dev)
> > +{
> > + return container_of(video_dev, struct virtio_media, video_dev);
> > +}
> > +
> > +/* virtio_media_driver.c */
> > +
> > +int virtio_media_send_command(struct virtio_media *vv, struct scatterlist **sgs,
> > + const size_t out_sgs, const size_t in_sgs,
> > + size_t minimum_resp_len, size_t *resp_len);
> > +void virtio_media_process_events(struct virtio_media *vv);
> > +
> > +/* virtio_media_ioctls.c */
> > +
> > +long virtio_media_device_ioctl(struct file *file, unsigned int cmd,
> > + unsigned long arg);
> > +extern const struct v4l2_ioctl_ops virtio_media_ioctl_ops;
> > +
> > +#endif // __VIRTIO_MEDIA_H
> > --
> > 2.55.0.rc0.799.gd6f94ed593-goog
>
^ permalink raw reply
* Re: [PATCH v4 6/8] media: virtio: Add virtio_media_driver
From: Brian Daniels @ 2026-07-17 20:58 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Mauro Carvalho Chehab, acourbot, adelva, aesteve, changyeon,
daniel.almeida, eperezma, gnurou, gurchetansingh, hverkuil,
jasowang, linux-kernel, linux-media, nicolas.dufresne,
virtualization, xuanzhuo
In-Reply-To: <20260625201850.2981130-1-briandaniels@google.com>
On Thu, Jun 25, 2026 at 4:18 PM Brian Daniels <briandaniels@google.com> wrote:
>
> > > From: Alexandre Courbot <gnurou@gmail.com>
> > >
> > > virtio_media_driver.c provides the expected driver hooks, and support
> > > for mmapping and polling.
> > >
> > > Signed-off-by: Alexandre Courbot <gnurou@gmail.com>
> > > Co-developed-by: Brian Daniels <briandaniels@google.com>
> > > Signed-off-by: Brian Daniels <briandaniels@google.com>
> > > ---
> > > drivers/media/virtio/virtio_media_driver.c | 959 +++++++++++++++++++++
> > > 1 file changed, 959 insertions(+)
> > > create mode 100644 drivers/media/virtio/virtio_media_driver.c
> > >
> > > diff --git a/drivers/media/virtio/virtio_media_driver.c b/drivers/media/virtio/virtio_media_driver.c
> > > new file mode 100644
> > > index 000000000..d6363c673
> > > --- /dev/null
> > > +++ b/drivers/media/virtio/virtio_media_driver.c
> > > @@ -0,0 +1,959 @@
> > > +// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0+
> > > +
> > > +/*
> > > + * Virtio-media driver.
> > > + *
> > > + * Copyright (c) 2024-2025 Google LLC.
> > > + */
> > > +
> > > +#include <linux/delay.h>
> > > +#include <linux/device.h>
> > > +#include <linux/dev_printk.h>
> > > +#include <linux/mm.h>
> > > +#include <linux/mutex.h>
> > > +#include <linux/scatterlist.h>
> > > +#include <linux/types.h>
> > > +#include <linux/videodev2.h>
> > > +#include <linux/vmalloc.h>
> > > +#include <linux/wait.h>
> > > +#include <linux/workqueue.h>
> > > +#include <linux/module.h>
> > > +#include <linux/moduleparam.h>
> > > +#include <linux/virtio.h>
> > > +#include <linux/virtio_config.h>
> > > +#include <linux/virtio_ids.h>
> > > +
> > > +#include <media/frame_vector.h>
> > > +#include <media/v4l2-dev.h>
> > > +#include <media/v4l2-event.h>
> > > +#include <media/videobuf2-memops.h>
> > > +#include <media/v4l2-device.h>
> > > +#include <media/v4l2-ioctl.h>
> > > +
> > > +#include "protocol.h"
> > > +#include "session.h"
> > > +#include "virtio_media.h"
> > > +
> > > +#define VIRTIO_MEDIA_NUM_EVENT_BUFS 16
> > > +
> > > +/* ID of the SHM region into which MMAP buffer will be mapped. */
> > > +#define VIRTIO_MEDIA_SHM_MMAP 0
> > > +
> > > +/*
> > > + * Name of the driver to expose to user-space.
> > > + *
> > > + * This is configurable because v4l2-compliance has workarounds specific to
> > > + * some drivers. When proxying these directly from the host, this allows it to
> > > + * apply them as needed.
> > > + */
> > > +char *virtio_media_driver_name;
> > > +module_param_named(driver_name, virtio_media_driver_name, charp, 0660);
> >
> >
> > Um. What? Not how it should be handled.
>
> I can remove this module param. I didn't end up using this when compliance testing.
> Instead, I patched v4l-utils:
> https://lore.kernel.org/all/20260528163448.4031965-1-briandaniels@google.com/
>
> Let me know if you think the v4l-utils patch is a good approach, otherwise let
> me know how you'd prefer to address the v4l2-compliance driver-specific workounds
> when they're being proxied with virtio-media.
>
> > > +
> > > +/*
> > > + * Whether USERPTR buffers are allowed.
> > > + *
> > > + * This is disabled by default as USERPTR buffers are dangerous, but the option
> > > + * is left to enable them if desired.
> > > + */
> > > +bool virtio_media_allow_userptr;
> > > +module_param_named(allow_userptr, virtio_media_allow_userptr, bool, 0660);
> >
> >
> > is this kind of thing common?
>
> To be honest, I don't really know. I'm also not that familiar with the USERPTR
> issues. I see a few references online about their use being discouraged due to
> possible race conditions, perhaps that was the original motivation for this
> parameter (I'm not the original author of this driver).
>
> I'm open to alternatives, feel free to let me know if you have a preference.
>
> > > +
> > > +/**
> > > + * virtio_media_session_alloc - Allocate a new session.
> > > + * @vv: virtio-media device the session belongs to.
> > > + * @id: ID of the session.
> > > + * @nonblocking_dequeue: whether dequeuing of buffers should be blocking or
> > > + * not.
> > > + *
> > > + * The ``id`` and ``list`` fields must still be set by the caller.
> >
> > still in what sense?
>
> Based on the code below, I'm not so sure that the caller is responsible for
> setting these values. They seem to be initialized in the function.
>
> Perhaps Alexandre Courbot (the original author) would know more. Unless he
> says otherwise though I'm inclined to remove this comment.
>
> > > + */
> > > +static struct virtio_media_session *
> > > +virtio_media_session_alloc(struct virtio_media *vv, u32 id,
> > > + struct file *file)
> > > +{
> > > + struct virtio_media_session *session;
> > > + int i;
> > > + int ret;
> > > +
> > > + session = kzalloc_obj(*session, GFP_KERNEL);
> > > + if (!session)
> > > + goto err_session;
> > > +
> > > + session->shadow_buf = kzalloc(VIRTIO_SHADOW_BUF_SIZE, GFP_KERNEL);
> > > + if (!session->shadow_buf)
> > > + goto err_shadow_buf;
> > > +
> > > + ret = sg_alloc_table(&session->command_sgs, DESC_CHAIN_MAX_LEN,
> > > + GFP_KERNEL);
> > > + if (ret)
> > > + goto err_payload_sgs;
> > > +
> > > + session->id = id;
> > > + session->nonblocking_dequeue = file->f_flags & O_NONBLOCK;
> > > +
> > > + INIT_LIST_HEAD(&session->list);
> > > + v4l2_fh_init(&session->fh, &vv->video_dev);
> > > + virtio_media_session_fh_add(session, file);
> > > +
> > > + for (i = 0; i <= VIRTIO_MEDIA_LAST_QUEUE; i++)
> > > + INIT_LIST_HEAD(&session->queues[i].pending_dqbufs);
> > > + mutex_init(&session->queues_lock);
> > > +
> > > + init_waitqueue_head(&session->dqbuf_wait);
> > > +
> > > + mutex_lock(&vv->sessions_lock);
> > > + list_add_tail(&session->list, &vv->sessions);
> > > + mutex_unlock(&vv->sessions_lock);
> > > +
> > > + return session;
> > > +
> > > +err_payload_sgs:
> > > + kfree(session->shadow_buf);
> > > +err_shadow_buf:
> > > + kfree(session);
> > > +err_session:
> > > + return ERR_PTR(-ENOMEM);
> > > +}
> > > +
> > > +/**
> > > + * virtio_media_session_free - Free all resources of a session.
> > > + * @vv: virtio-media device the session belongs to.
> > > + * @session: session to destroy.
> > > + *
> > > + * All the resources of @sesssion, as well as the backing memory of @session
> > > + * itself, are freed.
> >
> > why @ here and `` above? And typo in the name.
>
> The `@` here was an attempt to follow the guide here for referencing function
> parameters:
> https://docs.kernel.org/doc-guide/kernel-doc.html#highlights-and-cross-references
>
> That being said, I don't believe this file is 100% consistent with that. I will
> spend some time cleaning up the comments throughout this patch set to get them
> consistent for v5. Thanks!
>
> > > + */
> > > +static void virtio_media_session_free(struct virtio_media *vv,
> > > + struct virtio_media_session *session)
> > > +{
> > > + int i;
> > > +
> > > + mutex_lock(&vv->sessions_lock);
> > > + list_del(&session->list);
> > > + mutex_unlock(&vv->sessions_lock);
> > > +
> > > + virtio_media_session_fh_del(session);
> > > + v4l2_fh_exit(&session->fh);
> > > +
> > > + sg_free_table(&session->command_sgs);
> > > +
> > > + for (i = 0; i <= VIRTIO_MEDIA_LAST_QUEUE; i++)
> > > + vfree(session->queues[i].buffers);
> > > +
> > > + kfree(session->shadow_buf);
> > > + kfree(session);
> > > +}
> > > +
> > > +/**
> > > + * virtio_media_session_close - Close and free a session.
> > > + * @vv: virtio-media device the session belongs to.
> > > + * @session: session to close and destroy.
> > > + *
> > > + * This send the ``VIRTIO_MEDIA_CMD_CLOSE`` command to the device, and frees
> >
> > sends
>
> Fix staged in v5
>
> > > + * all resources used by @session.
> > > + */
> > > +static int virtio_media_session_close(struct virtio_media *vv,
> > > + struct virtio_media_session *session)
> > > +{
> > > + struct virtio_media_cmd_close *cmd_close = &session->cmd.close;
> > > + struct scatterlist cmd_sg = {};
> > > + struct scatterlist *sgs[1] = { &cmd_sg };
> > > + int ret;
> > > +
> > > + mutex_lock(&vv->vlock);
> > > +
> > > + cmd_close->hdr.cmd = VIRTIO_MEDIA_CMD_CLOSE;
> > > + cmd_close->session_id = session->id;
> > > +
> > > + sg_set_buf(&cmd_sg, cmd_close, sizeof(*cmd_close));
> > > + sg_mark_end(&cmd_sg);
> > > +
> > > + ret = virtio_media_send_command(vv, sgs, 1, 0, 0, NULL);
> > > + mutex_unlock(&vv->vlock);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + virtio_media_session_free(vv, session);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +/**
> > > + * virtio_media_find_session - Lookup for the session with a given ID.
> >
> > a session
>
> Fix staged in v5
>
> > > + * @vv: virtio-media device to lookup the session from.
> > > + * @id: ID of the session to lookup.
> > > + */
> > > +static struct virtio_media_session *
> > > +virtio_media_find_session(struct virtio_media *vv, u32 id)
> > > +{
> > > + struct list_head *p;
> > > + struct virtio_media_session *session = NULL;
> > > +
> > > + mutex_lock(&vv->sessions_lock);
> > > + list_for_each(p, &vv->sessions) {
> > > + struct virtio_media_session *s =
> > > + list_entry(p, struct virtio_media_session, list);
> > > + if (s->id == id) {
> > > + session = s;
> > > + break;
> > > + }
> > > + }
> > > + mutex_unlock(&vv->sessions_lock);
> > > +
> > > + return session;
> > > +}
> > > +
> > > +/**
> > > + * struct virtio_media_cmd_callback_param - Callback parameters to the virtio
> > > + * command queue.
> > > + * @vv: virtio-media device in use.
> > > + * @done: flag to be switched once the command is completed.
> > > + * @resp_len: length of the received response from the command. Only valid
> > > + * after @done_flag has switched to ``true``.
> >
> > confusing indent
>
> Fix staged in v5
>
> > > + */
> > > +struct virtio_media_cmd_callback_param {
> > > + struct virtio_media *vv;
> > > + bool done;
> > > + size_t resp_len;
> > > +};
> > > +
> > > +/**
> > > + * commandq_callback: Callback for the command queue.
> > > + * @queue: command virtqueue.
> > > + *
> > > + * This just wakes up the thread that was waiting on the command to complete.
> > > + */
> > > +static void commandq_callback(struct virtqueue *queue)
> > > +{
> > > + unsigned int len;
> > > + struct virtio_media_cmd_callback_param *param;
> > > +
> > > +process_bufs:
> > > + while ((param = virtqueue_get_buf(queue, &len))) {
> > > + param->done = true;
> > > + param->resp_len = len;
> > > + wake_up(¶m->vv->wq);
> > > + }
> > > +
> > > + if (!virtqueue_enable_cb(queue)) {
> > > + virtqueue_disable_cb(queue);
> > > + goto process_bufs;
> > > + }
> > > +}
> > > +
> > > +/**
> > > + * virtio_media_kick_command - send a command to the commandq.
> > > + * @vv: virtio-media device in use.
> > > + * @sgs: descriptor chain to send.
> > > + * @out_sgs: number of device-readable descriptors in @sgs.
> > > + * @in_sgs: number of device-writable descriptors in @sgs.
> > > + * @resp_len: output parameter. Upon success, contains the size of the response
> > > + * in bytes.
> >
> > confusing indent
> >
> > > + *
> >
> > why an empty line?
>
> Indent fixed and empty line removed in v5
>
> > > + */
> > > +static int virtio_media_kick_command(struct virtio_media *vv,
> > > + struct scatterlist **sgs,
> > > + const size_t out_sgs, const size_t in_sgs,
> > > + size_t *resp_len)
> > > +{
> > > + struct virtio_media_cmd_callback_param cb_param = {
> > > + .vv = vv,
> > > + .done = false,
> > > + .resp_len = 0,
> > > + };
> > > + struct virtio_media_resp_header *resp_header;
> > > + int ret;
> > > +
> > > + ret = virtqueue_add_sgs(vv->commandq, sgs, out_sgs, in_sgs, &cb_param,
> > > + GFP_ATOMIC);
> >
> >
> > can init with declaration.
>
> Thanks, added to v5
>
> > > + if (ret) {
> > > + v4l2_err(&vv->v4l2_dev,
> > > + "failed to add sgs to command virtqueue\n");
> > > + return ret;
> > > + }
> > > +
> > > + if (!virtqueue_kick(vv->commandq)) {
> > > + v4l2_err(&vv->v4l2_dev, "failed to kick command virtqueue\n");
> > > + return -EINVAL;
> > > + }
> > > +
> > > + /* Wait for the response. */
> > > + ret = wait_event_timeout(vv->wq, cb_param.done, 5 * HZ);
> > > + if (ret == 0) {
> > > + v4l2_err(&vv->v4l2_dev,
> > > + "timed out waiting for response to command\n");
> > > + return -ETIMEDOUT;
> > > + }
> > > +
> > > + if (resp_len)
> > > + *resp_len = cb_param.resp_len;
> > > +
> > > + if (in_sgs > 0) {
> > > + /*
> > > + * If we expect a response, make sure we have at least a
> > > + * response header - anything shorter is invalid.
> > > + */
> > > + if (cb_param.resp_len < sizeof(*resp_header)) {
> > > + v4l2_err(&vv->v4l2_dev,
> > > + "received response header is too short\n");
> > > + return -EINVAL;
> > > + }
> > > +
> > > + resp_header = sg_virt(sgs[out_sgs]);
> > > + if (resp_header->status)
> > > + /* Host returns a positive error code. */
> > > + return -resp_header->status;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +/**
> > > + * virtio_media_send_command - Send a command to the device and wait for its
> > > + * response.
> > > + * @vv: virtio-media device in use.
> > > + * @sgs: descriptor chain to send.
> > > + * @out_sgs: number of device-readable descriptors in @sgs.
> > > + * @in_sgs: number of device-writable descriptors in @sgs.
> > > + * @minimum_resp_len: minimum length of the response expected by the caller
> > > + * when the command is successful. Anything shorter than that will result in
> > > + * ``-EINVAL`` being returned.
> > > + * @resp_len: output parameter. Upon success, contains the size of the response
> > > + * in bytes.
> > > + */
> > > +int virtio_media_send_command(struct virtio_media *vv, struct scatterlist **sgs,
> > > + const size_t out_sgs, const size_t in_sgs,
> > > + size_t minimum_resp_len, size_t *resp_len)
> > > +{
> > > + size_t local_resp_len = resp_len ? *resp_len : 0;
> > > + int ret = virtio_media_kick_command(vv, sgs, out_sgs, in_sgs,
> > > + &local_resp_len);
> > > + if (resp_len)
> > > + *resp_len = local_resp_len;
> > > +
> > > + /*
> > > + * If the host could not process the command, there is no valid
> > > + * response.
> > > + */
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + /* Make sure the host wrote a complete reply. */
> > > + if (local_resp_len < minimum_resp_len) {
> > > + v4l2_err(&vv->v4l2_dev,
> > > + "received response is too short: received %zu, expected at least %zu\n",
> > > + local_resp_len, minimum_resp_len);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +/**
> > > + * virtio_media_send_event_buffer() - Sends an event buffer to the host so it
> > > + * can return it with an event.
> > > + * @vv: virtio-media device in use.
> > > + * @event_buffer: pointer to the event buffer to send to the device.
> > > + */
> > > +static int virtio_media_send_event_buffer(struct virtio_media *vv,
> > > + void *event_buffer)
> > > +{
> > > + struct scatterlist *sgs[1], vresp;
> > > + int ret;
> > > +
> > > + sg_init_one(&vresp, event_buffer, VIRTIO_MEDIA_EVENT_MAX_SIZE);
> > > + sgs[0] = &vresp;
> >
> > what is this convoluted thing? why not pass &vresp directly?
>
> `virtqueue_add_sgs` expects a `struct scatterlist **`. Passing `&vresp` is
> only a single pointer and it needs a double pointer.
>
> It could be written as:
>
> struct scatterlist *sgs, vresp;
>
> sg_init_one(&vresp, event_buffer, VIRTIO_MEDIA_EVENT_MAX_SIZE);
> sgs = &vresp;
> ret = virtqueue_add_sgs(vv->eventq, &sgs, 0, 1, event_buffer,
>
> But I suspect it's written as is to match `virtqueue_add_sgs`'s function
> declaration:
>
> int virtqueue_add_sgs(struct virtqueue *vq,
> struct scatterlist *sgs[],
> unsigned int out_sgs,
> unsigned int in_sgs,
> void *data,
> gfp_t gfp);
>
> > > +
> > > + ret = virtqueue_add_sgs(vv->eventq, sgs, 0, 1, event_buffer,
> > > + GFP_ATOMIC);
> >
> >
> > This does not work uness event_buffer is aligned for dma.
> > But it does not seem to be:
> >
> > for (i = 0; i < VIRTIO_MEDIA_NUM_EVENT_BUFS; i++) {
> > void *ebuf = vv->event_buffer + VIRTIO_MEDIA_EVENT_MAX_SIZE * i;
> >
> > ret = virtio_media_send_event_buffer(vv, ebuf);
> > if (ret)
> > goto err_send_event_buffer;
>
> Apologies if the following questions are bit basic, I'm still pretty new to
> this.
>
> Is the dma alignment requirement a general requirement for calling the
> `virtqueue_add_sgs` function? Or is the dma alignment requirement to allow the
> driver to support DMABUF transfers? If its the later, I should note this driver
> does not support DMABUF transfers yet when streaming frames.
>
> If it's the former, then how would I go about ensuring its dma aligned?
>
> From the code block you quoted above, `vv->event_buffer` is allocated like so:
>
> vv->event_buffer = devm_kzalloc(dev,
> VIRTIO_MEDIA_EVENT_MAX_SIZE *
> VIRTIO_MEDIA_NUM_EVENT_BUFS,
> GFP_KERNEL);
> if (!vv->event_buffer)
> return -ENOMEM;
>
> Do I need to call something else besides `devm_kzalloc()` to ensure every
> element within the vv->event_buffer array is dma aligned?
Before I send out v5, would you mind addressing this question? I
didn't want it getting lost in all of the threads. Thanks!
> >
> >
> > > + if (ret) {
> > > + v4l2_err(&vv->v4l2_dev,
> > > + "failed to add sgs to event virtqueue\n");
> > > + return ret;
> > > + }
> > > +
> > > + if (!virtqueue_kick(vv->eventq)) {
> > > + v4l2_err(&vv->v4l2_dev, "failed to kick event virtqueue\n");
> > > + return -EINVAL;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +/**
> > > + * eventq_callback() - Callback for the event queue.
> > > + * @queue: event virtqueue.
> > > + *
> > > + * This just schedules for event work to be run.
> > > + */
> > > +static void eventq_callback(struct virtqueue *queue)
> > > +{
> > > + struct virtio_media *vv = queue->vdev->priv;
> > > +
> > > + schedule_work(&vv->eventq_work);
> > > +}
> > > +
> > > +/**
> > > + * virtio_media_process_dqbuf_event() - Process a dequeued event for a session.
> > > + * @vv: virtio-media device in use.
> > > + * @session: session the event is addressed to.
> > > + * @dqbuf_evt: the dequeued event to process.
> > > + *
> > > + * Invalid events are ignored with an error log.
> > > + */
> > > +static void
> > > +virtio_media_process_dqbuf_event(struct virtio_media *vv,
> > > + struct virtio_media_session *session,
> > > + struct virtio_media_event_dqbuf *dqbuf_evt)
> > > +{
> > > + struct virtio_media_buffer *dqbuf;
> > > + const enum v4l2_buf_type queue_type = dqbuf_evt->buffer.type;
> > > + struct virtio_media_queue_state *queue;
> > > + typeof(dqbuf->buffer.m) buffer_m;
> > > + typeof(dqbuf->buffer.m.planes[0].m) plane_m;
> > > + int i;
> > > +
> > > + if (queue_type >= ARRAY_SIZE(session->queues)) {
> > > + v4l2_err(&vv->v4l2_dev,
> > > + "unmanaged queue %d passed to dqbuf event",
> > > + dqbuf_evt->buffer.type);
> > > + return;
> > > + }
> > > + queue = &session->queues[queue_type];
> > > +
> > > + if (dqbuf_evt->buffer.index >= queue->allocated_bufs) {
> > > + v4l2_err(&vv->v4l2_dev,
> > > + "invalid buffer ID %d for queue %d in dqbuf event",
> > > + dqbuf_evt->buffer.index, dqbuf_evt->buffer.type);
> > > + return;
> > > + }
> > > +
> > > + dqbuf = &queue->buffers[dqbuf_evt->buffer.index];
> > > +
> > > + /*
> > > + * Preserve the 'm' union that was passed to us during QBUF so userspace
> > > + * gets back the information it submitted.
> > > + */
> > > + buffer_m = dqbuf->buffer.m;
> > > + memcpy(&dqbuf->buffer, &dqbuf_evt->buffer, sizeof(dqbuf->buffer));
> > > + dqbuf->buffer.m = buffer_m;
> > > + if (V4L2_TYPE_IS_MULTIPLANAR(dqbuf->buffer.type)) {
> > > + if (dqbuf->buffer.length > VIDEO_MAX_PLANES) {
> > > + v4l2_err(&vv->v4l2_dev,
> > > + "invalid number of planes received from host for a multiplanar buffer\n");
> > > + return;
> > > + }
> > > + for (i = 0; i < dqbuf->buffer.length; i++) {
> > > + plane_m = dqbuf->planes[i].m;
> > > + memcpy(&dqbuf->planes[i], &dqbuf_evt->planes[i],
> > > + sizeof(struct v4l2_plane));
> > > + dqbuf->planes[i].m = plane_m;
> > > + }
> > > + }
> > > +
> > > + /* Set the DONE flag as the buffer is waiting for being dequeued. */
> > > + dqbuf->buffer.flags |= V4L2_BUF_FLAG_DONE;
> > > +
> > > + mutex_lock(&session->queues_lock);
> > > + list_add_tail(&dqbuf->list, &queue->pending_dqbufs);
> > > + queue->queued_bufs -= 1;
> > > + mutex_unlock(&session->queues_lock);
> > > +
> > > + wake_up(&session->dqbuf_wait);
> > > +}
> > > +
> > > +/**
> > > + * virtio_media_process_events() - Process all pending events on a device.
> > > + * @vv: device which pending events we want to process.
> > > + *
> > > + * Retrieves all pending events on @vv's event queue and dispatch them to their
> > > + * corresponding session.
> > > + *
> > > + * Invalid events are ignored with an error log.
> > > + */
> > > +void virtio_media_process_events(struct virtio_media *vv)
> > > +{
> > > + struct virtio_media_event_error *error_evt;
> > > + struct virtio_media_event_dqbuf *dqbuf_evt;
> > > + struct virtio_media_event_event *event_evt;
> > > + struct virtio_media_session *session;
> > > + struct virtio_media_event_header *evt;
> > > + unsigned int len;
> > > +
> > > + mutex_lock(&vv->events_lock);
> > > +
> > > +process_bufs:
> > > + while ((evt = virtqueue_get_buf(vv->eventq, &len))) {
> > > + /* Make sure we received enough data */
> > > + if (len < sizeof(*evt)) {
> > > + v4l2_err(&vv->v4l2_dev,
> > > + "event is too short: got %u, expected at least %zu\n",
> > > + len, sizeof(*evt));
> > > + goto end_of_event;
> > > + }
> > > +
> > > + session = virtio_media_find_session(vv, evt->session_id);
> > > + if (!session) {
> > > + v4l2_err(&vv->v4l2_dev, "cannot find session %d\n",
> > > + evt->session_id);
> > > + goto end_of_event;
> > > + }
> > > +
> > > + switch (evt->event) {
> > > + case VIRTIO_MEDIA_EVT_ERROR:
> > > + if (len < sizeof(*error_evt)) {
> > > + v4l2_err(&vv->v4l2_dev,
> > > + "error event is too short: got %u, expected %zu\n",
> > > + len, sizeof(*error_evt));
> > > + break;
> > > + }
> > > + error_evt = (struct virtio_media_event_error *)evt;
> > > + v4l2_err(&vv->v4l2_dev,
> > > + "received error %d for session %d",
> > > + error_evt->errno, error_evt->hdr.session_id);
> > > + virtio_media_session_close(vv, session);
> > > + break;
> > > +
> > > + /*
> > > + * Dequeued buffer: put it into the right queue so user-space
> > > + * can dequeue it.
> > > + */
> > > + case VIRTIO_MEDIA_EVT_DQBUF:
> > > + if (len < sizeof(*dqbuf_evt)) {
> > > + v4l2_err(&vv->v4l2_dev,
> > > + "dqbuf event is too short: got %u, expected %zu\n",
> > > + len, sizeof(*dqbuf_evt));
> > > + break;
> > > + }
> > > + dqbuf_evt = (struct virtio_media_event_dqbuf *)evt;
> > > + virtio_media_process_dqbuf_event(vv, session,
> > > + dqbuf_evt);
> > > + break;
> > > +
> > > + case VIRTIO_MEDIA_EVT_EVENT:
> > > + if (len < sizeof(*event_evt)) {
> > > + v4l2_err(&vv->v4l2_dev,
> > > + "session event is too short: got %u expected %zu\n",
> > > + len, sizeof(*event_evt));
> > > + break;
> > > + }
> > > +
> > > + event_evt = (struct virtio_media_event_event *)evt;
> > > + v4l2_event_queue_fh(&session->fh, &event_evt->event);
> > > + break;
> > > +
> > > + default:
> > > + v4l2_err(&vv->v4l2_dev, "unknown event type %d\n",
> > > + evt->event);
> > > + break;
> > > + }
> > > +
> > > +end_of_event:
> > > + virtio_media_send_event_buffer(vv, evt);
> > > + }
> > > +
> > > + if (!virtqueue_enable_cb(vv->eventq)) {
> > > + virtqueue_disable_cb(vv->eventq);
> > > + goto process_bufs;
> > > + }
> > > +
> > > + mutex_unlock(&vv->events_lock);
> > > +}
> > > +
> > > +static void virtio_media_event_work(struct work_struct *work)
> > > +{
> > > + struct virtio_media *vv =
> > > + container_of(work, struct virtio_media, eventq_work);
> > > +
> > > + virtio_media_process_events(vv);
> > > +}
> > > +
> > > +/**
> > > + * virtio_media_device_open() - Create a new session from an opened file.
> > > + * @file: opened file for the session.
> > > + */
> > > +static int virtio_media_device_open(struct file *file)
> > > +{
> > > + struct video_device *video_dev = video_devdata(file);
> > > + struct virtio_media *vv = to_virtio_media(video_dev);
> > > + struct virtio_media_cmd_open *cmd_open = &vv->cmd.open;
> > > + struct virtio_media_resp_open *resp_open = &vv->resp.open;
> > > + struct scatterlist cmd_sg = {}, resp_sg = {};
> > > + struct scatterlist *sgs[2] = { &cmd_sg, &resp_sg };
> > > + struct virtio_media_session *session;
> > > + u32 session_id;
> > > + int ret;
> > > +
> > > + mutex_lock(&vv->vlock);
> > > +
> > > + sg_set_buf(&cmd_sg, cmd_open, sizeof(*cmd_open));
> > > + sg_mark_end(&cmd_sg);
> > > +
> > > + sg_set_buf(&resp_sg, resp_open, sizeof(*resp_open));
> > > + sg_mark_end(&resp_sg);
> > > +
> > > + cmd_open->hdr.cmd = VIRTIO_MEDIA_CMD_OPEN;
> > > + ret = virtio_media_send_command(vv, sgs, 1, 1, sizeof(*resp_open),
> > > + NULL);
> > > + session_id = resp_open->session_id;
> > > + mutex_unlock(&vv->vlock);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + session = virtio_media_session_alloc(vv, session_id, file);
> > > + if (IS_ERR(session))
> > > + return PTR_ERR(session);
> > > +
> > > + file->private_data = &session->fh;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +/**
> > > + * virtio_media_device_close() - Close a previously opened session.
> > > + * @file: file of the session to close.
> > > + *
> > > + * This sends to ``VIRTIO_MEDIA_CMD_CLOSE`` command to the device, and close
> > > + * the session on the driver side.
> > > + */
> > > +static int virtio_media_device_close(struct file *file)
> > > +{
> > > + struct video_device *video_dev = video_devdata(file);
> > > + struct virtio_media *vv = to_virtio_media(video_dev);
> > > + struct virtio_media_session *session =
> > > + fh_to_session(file->private_data);
> > > +
> > > + return virtio_media_session_close(vv, session);
> > > +}
> > > +
> > > +/**
> > > + * virtio_media_device_poll() - Poll logic for a virtio-media device.
> > > + * @file: file of the session to poll.
> > > + * @wait: poll table to wait on.
> > > + */
> > > +static __poll_t virtio_media_device_poll(struct file *file, poll_table *wait)
> > > +{
> > > + struct virtio_media_session *session =
> > > + fh_to_session(file->private_data);
> > > + enum v4l2_buf_type capture_type =
> > > + session->uses_mplane ? V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE :
> > > + V4L2_BUF_TYPE_VIDEO_CAPTURE;
> > > + enum v4l2_buf_type output_type =
> > > + session->uses_mplane ? V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE :
> > > + V4L2_BUF_TYPE_VIDEO_OUTPUT;
> > > + struct virtio_media_queue_state *capture_queue =
> > > + &session->queues[capture_type];
> > > + struct virtio_media_queue_state *output_queue =
> > > + &session->queues[output_type];
> > > + __poll_t req_events = poll_requested_events(wait);
> > > + __poll_t rc = 0;
> > > +
> > > + poll_wait(file, &session->dqbuf_wait, wait);
> > > + poll_wait(file, &session->fh.wait, wait);
> > > +
> > > + mutex_lock(&session->queues_lock);
> > > + if (req_events & (EPOLLIN | EPOLLRDNORM)) {
> > > + if (!capture_queue->streaming ||
> > > + (capture_queue->queued_bufs == 0 &&
> > > + list_empty(&capture_queue->pending_dqbufs)))
> > > + rc |= EPOLLERR;
> > > + else if (!list_empty(&capture_queue->pending_dqbufs))
> > > + rc |= EPOLLIN | EPOLLRDNORM;
> > > + }
> > > + if (req_events & (EPOLLOUT | EPOLLWRNORM)) {
> > > + if (!output_queue->streaming)
> > > + rc |= EPOLLERR;
> > > + else if (output_queue->queued_bufs <
> > > + output_queue->allocated_bufs)
> > > + rc |= EPOLLOUT | EPOLLWRNORM;
> > > + }
> > > + mutex_unlock(&session->queues_lock);
> > > +
> > > + if (v4l2_event_pending(&session->fh))
> > > + rc |= EPOLLPRI;
> > > +
> > > + return rc;
> > > +}
> > > +
> > > +static void virtio_media_vma_close_locked(struct vm_area_struct *vma)
> > > +{
> > > + struct virtio_media *vv = vma->vm_private_data;
> > > + struct virtio_media_cmd_munmap *cmd_munmap = &vv->cmd.munmap;
> > > + struct virtio_media_resp_munmap *resp_munmap = &vv->resp.munmap;
> > > + struct scatterlist cmd_sg = {}, resp_sg = {};
> > > + struct scatterlist *sgs[2] = { &cmd_sg, &resp_sg };
> > > + int ret;
> > > +
> > > + sg_set_buf(&cmd_sg, cmd_munmap, sizeof(*cmd_munmap));
> > > + sg_mark_end(&cmd_sg);
> > > +
> > > + sg_set_buf(&resp_sg, resp_munmap, sizeof(*resp_munmap));
> > > + sg_mark_end(&resp_sg);
> > > +
> > > + cmd_munmap->hdr.cmd = VIRTIO_MEDIA_CMD_MUNMAP;
> > > + cmd_munmap->driver_addr =
> > > + (vma->vm_pgoff << PAGE_SHIFT) - vv->mmap_region.addr;
> > > + ret = virtio_media_send_command(vv, sgs, 1, 1, sizeof(*resp_munmap),
> > > + NULL);
> > > + if (ret < 0) {
> > > + v4l2_err(&vv->v4l2_dev, "host failed to unmap buffer: %d\n",
> > > + ret);
> > > + }
> > > +}
> > > +
> > > +/**
> > > + * virtio_media_vma_close() - Close a MMAP buffer mapping.
> > > + * @vma: VMA of the mapping to close.
> > > + *
> > > + * Inform the host that a previously created MMAP mapping is no longer needed
> > > + * and can be removed.
> > > + */
> > > +static void virtio_media_vma_close(struct vm_area_struct *vma)
> > > +{
> > > + struct virtio_media *vv = vma->vm_private_data;
> > > +
> > > + mutex_lock(&vv->vlock);
> > > + virtio_media_vma_close_locked(vma);
> > > + mutex_unlock(&vv->vlock);
> > > +}
> > > +
> > > +static const struct vm_operations_struct virtio_media_vm_ops = {
> > > + .close = virtio_media_vma_close,
> > > +};
> > > +
> > > +/**
> > > + * virtio_media_device_mmap - Perform a mmap request from userspace.
> > > + * @file: opened file of the session to map for.
> > > + * @vma: VM area struct describing the desired mapping.
> > > + *
> > > + * This requests the host to map a MMAP buffer for us, so we can then make that
> > > + * mapping visible into user-space address space.
> > > + */
> > > +static int virtio_media_device_mmap(struct file *file,
> > > + struct vm_area_struct *vma)
> > > +{
> > > + struct video_device *video_dev = video_devdata(file);
> > > + struct virtio_media *vv = to_virtio_media(video_dev);
> > > + struct virtio_media_session *session =
> > > + fh_to_session(file->private_data);
> > > + struct virtio_media_cmd_mmap *cmd_mmap = &session->cmd.mmap;
> > > + struct virtio_media_resp_mmap *resp_mmap = &session->resp.mmap;
> > > + struct scatterlist cmd_sg = {}, resp_sg = {};
> > > + struct scatterlist *sgs[2] = { &cmd_sg, &resp_sg };
> > > + int ret;
> > > +
> > > + if (!(vma->vm_flags & VM_SHARED))
> > > + return -EINVAL;
> > > + if (!(vma->vm_flags & (VM_READ | VM_WRITE)))
> > > + return -EINVAL;
> > > +
> > > + mutex_lock(&vv->vlock);
> > > +
> > > + cmd_mmap->hdr.cmd = VIRTIO_MEDIA_CMD_MMAP;
> > > + cmd_mmap->session_id = session->id;
> > > + cmd_mmap->flags =
> > > + (vma->vm_flags & VM_WRITE) ? VIRTIO_MEDIA_MMAP_FLAG_RW : 0;
> > > + cmd_mmap->offset = vma->vm_pgoff << PAGE_SHIFT;
> > > +
> > > + sg_set_buf(&cmd_sg, cmd_mmap, sizeof(*cmd_mmap));
> > > + sg_mark_end(&cmd_sg);
> > > +
> > > + sg_set_buf(&resp_sg, resp_mmap, sizeof(*resp_mmap));
> > > + sg_mark_end(&resp_sg);
> > > +
> > > + /*
> > > + * The host performs reference counting and is smart enough to return
> > > + * the same guest physical address if this is called several times on
> > > + * the same
> > > + * buffer.
> > > + */
> > > + ret = virtio_media_send_command(vv, sgs, 1, 1, sizeof(*resp_mmap),
> > > + NULL);
> > > + if (ret < 0)
> > > + goto end;
> > > +
> > > + vma->vm_private_data = vv;
> > > + /*
> > > + * Keep the guest address at which the buffer is mapped since we will
> > > + * use that to unmap.
> > > + */
> > > + vma->vm_pgoff = (resp_mmap->driver_addr + vv->mmap_region.addr) >>
> > > + PAGE_SHIFT;
> > > +
> > > + /*
> > > + * We cannot let the mapping be larger than the buffer.
> > > + */
> > > + if (vma->vm_end - vma->vm_start > PAGE_ALIGN(resp_mmap->len)) {
> > > + dev_dbg(&video_dev->dev,
> > > + "invalid MMAP, as it would overflow buffer length\n");
> > > + virtio_media_vma_close_locked(vma);
> > > + ret = -EINVAL;
> > > + goto end;
> > > + }
> > > +
> > > + ret = io_remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
> > > + vma->vm_end - vma->vm_start,
> > > + vma->vm_page_prot);
> > > + if (ret)
> > > + goto end;
> > > +
> > > + vma->vm_ops = &virtio_media_vm_ops;
> > > +
> > > +end:
> > > + mutex_unlock(&vv->vlock);
> > > + return ret;
> > > +}
> > > +
> > > +static const struct v4l2_file_operations virtio_media_fops = {
> > > + .owner = THIS_MODULE,
> > > + .open = virtio_media_device_open,
> > > + .release = virtio_media_device_close,
> > > + .poll = virtio_media_device_poll,
> > > + .unlocked_ioctl = virtio_media_device_ioctl,
> > > + .mmap = virtio_media_device_mmap,
> > > +};
> > > +
> > > +static int virtio_media_probe(struct virtio_device *virtio_dev)
> > > +{
> > > + struct device *dev = &virtio_dev->dev;
> > > + struct virtqueue *vqs[2];
> > > + static struct virtqueue_info vq_info[2] = {
> > > + {
> > > + .name = "command",
> > > + .callback = commandq_callback,
> > > + },
> > > + {
> > > + .name = "event",
> > > + .callback = eventq_callback,
> > > + },
> > > + };
> > > + struct virtio_media *vv;
> > > + struct video_device *vd;
> > > + int i;
> > > + int ret;
> > > +
> > > + vv = devm_kzalloc(dev, sizeof(*vv), GFP_KERNEL);
> > > + if (!vv)
> > > + return -ENOMEM;
> > > +
> > > + vv->event_buffer = devm_kzalloc(dev,
> > > + VIRTIO_MEDIA_EVENT_MAX_SIZE *
> > > + VIRTIO_MEDIA_NUM_EVENT_BUFS,
> > > + GFP_KERNEL);
> > > + if (!vv->event_buffer)
> > > + return -ENOMEM;
> > > +
> > > + INIT_LIST_HEAD(&vv->sessions);
> > > + mutex_init(&vv->sessions_lock);
> > > + mutex_init(&vv->events_lock);
> > > + mutex_init(&vv->vlock);
> > > +
> > > + vv->virtio_dev = virtio_dev;
> > > + virtio_dev->priv = vv;
> > > +
> > > + init_waitqueue_head(&vv->wq);
> > > +
> > > + ret = v4l2_device_register(dev, &vv->v4l2_dev);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + ret = virtio_find_vqs(virtio_dev, 2, vqs, vq_info, NULL);
> > > + if (ret)
> > > + goto err_find_vqs;
> > > +
> > > + vv->commandq = vqs[0];
> > > + vv->eventq = vqs[1];
> > > + INIT_WORK(&vv->eventq_work, virtio_media_event_work);
> > > +
> > > + /* Get MMAP buffer mapping SHM region */
> > > + virtio_get_shm_region(virtio_dev, &vv->mmap_region,
> > > + VIRTIO_MEDIA_SHM_MMAP);
> > > +
> > > + vd = &vv->video_dev;
> > > +
> > > + vd->v4l2_dev = &vv->v4l2_dev;
> > > + vd->vfl_type = VFL_TYPE_VIDEO;
> > > + vd->ioctl_ops = &virtio_media_ioctl_ops;
> > > + vd->fops = &virtio_media_fops;
> > > + vd->device_caps = virtio_cread32(virtio_dev, 0);
> > > + if (vd->device_caps & (V4L2_CAP_VIDEO_M2M | V4L2_CAP_VIDEO_M2M_MPLANE))
> > > + vd->vfl_dir = VFL_DIR_M2M;
> > > + else if (vd->device_caps &
> > > + (V4L2_CAP_VIDEO_OUTPUT | V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE))
> > > + vd->vfl_dir = VFL_DIR_TX;
> > > + else
> > > + vd->vfl_dir = VFL_DIR_RX;
> > > + vd->release = video_device_release_empty;
> > > + strscpy(vd->name, "virtio-media", sizeof(vd->name));
> > > +
> > > + video_set_drvdata(vd, vv);
> > > +
> > > + ret = video_register_device(vd, virtio_cread32(virtio_dev, 4), 0);
> > > + if (ret)
> > > + goto err_register_device;
> > > +
> > > + for (i = 0; i < VIRTIO_MEDIA_NUM_EVENT_BUFS; i++) {
> > > + void *ebuf = vv->event_buffer + VIRTIO_MEDIA_EVENT_MAX_SIZE * i;
> > > +
> > > + ret = virtio_media_send_event_buffer(vv, ebuf);
> > > + if (ret)
> > > + goto err_send_event_buffer;
> > > + }
> > > +
> > > + virtio_device_ready(virtio_dev);
> > > +
> > > + return 0;
> > > +
> > > +err_send_event_buffer:
> > > + video_unregister_device(&vv->video_dev);
> > > +err_register_device:
> > > + virtio_dev->config->del_vqs(virtio_dev);
> > > +err_find_vqs:
> > > + v4l2_device_unregister(&vv->v4l2_dev);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static void virtio_media_remove(struct virtio_device *virtio_dev)
> > > +{
> > > + struct virtio_media *vv = virtio_dev->priv;
> > > + struct list_head *p, *n;
> > > +
> > > + cancel_work_sync(&vv->eventq_work);
> > > + virtio_reset_device(virtio_dev);
> > > +
> > > + v4l2_device_unregister(&vv->v4l2_dev);
> > > + virtio_dev->config->del_vqs(virtio_dev);
> > > + video_unregister_device(&vv->video_dev);
> > > +
> > > + list_for_each_safe(p, n, &vv->sessions) {
> > > + struct virtio_media_session *s =
> > > + list_entry(p, struct virtio_media_session, list);
> > > +
> > > + virtio_media_session_free(vv, s);
> > > + }
> > > +}
> > > +
> > > +static struct virtio_device_id id_table[] = {
> > > + { VIRTIO_ID_MEDIA, VIRTIO_DEV_ANY_ID },
> > > + { 0 },
> > > +};
> > > +
> > > +static unsigned int features[] = {};
> > > +
> > > +static struct virtio_driver virtio_media_driver = {
> > > + .feature_table = features,
> > > + .feature_table_size = ARRAY_SIZE(features),
> > > + .driver.name = VIRTIO_MEDIA_DEFAULT_DRIVER_NAME,
> > > + .driver.owner = THIS_MODULE,
> > > + .id_table = id_table,
> > > + .probe = virtio_media_probe,
> > > + .remove = virtio_media_remove,
> > > +};
> > > +
> > > +module_virtio_driver(virtio_media_driver);
> > > +
> > > +MODULE_DEVICE_TABLE(virtio, id_table);
> > > +MODULE_DESCRIPTION("virtio media driver");
> > > +MODULE_AUTHOR("Alexandre Courbot <gnurou@gmail.com>");
> > > +MODULE_LICENSE("Dual BSD/GPL");
^ permalink raw reply
* Re: [PATCH v2 1/4] virtio-mem: validate device-reported block size
From: Carlos Bilbao @ 2026-07-18 3:31 UTC (permalink / raw)
To: Michael S. Tsirkin, Greg Kroah-Hartman
Cc: David Hildenbrand (Arm), Hari Mishal, Jason Wang, Xuan Zhuo,
Eugenio Pérez, virtualization, linux-kernel, elena.reshetova
In-Reply-To: <20260717085838-mutt-send-email-mst@kernel.org>
Hey there,
On 7/17/26 06:08, Michael S. Tsirkin wrote:
> On Fri, Jul 17, 2026 at 02:07:50PM +0200, Greg Kroah-Hartman wrote:
>> On Fri, Jul 17, 2026 at 06:52:46AM -0400, Michael S. Tsirkin wrote:
>>> On Fri, Jul 17, 2026 at 12:46:52PM +0200, Greg Kroah-Hartman wrote:
>>>> On Fri, Jul 17, 2026 at 06:23:57AM -0400, Michael S. Tsirkin wrote:
>>>>> On Fri, Jul 17, 2026 at 12:15:09PM +0200, Greg Kroah-Hartman wrote:
>>>>>> On Fri, Jul 17, 2026 at 06:10:41AM -0400, Michael S. Tsirkin wrote:
>>>>>>> On Fri, Jul 17, 2026 at 11:14:23AM +0200, Greg Kroah-Hartman wrote:
>>>>>>>> On Fri, Jul 17, 2026 at 04:59:32AM -0400, Michael S. Tsirkin wrote:
>>>>>>>>> On Fri, Jul 17, 2026 at 10:39:40AM +0200, David Hildenbrand (Arm) wrote:
>>>>>>>>>> On 7/17/26 07:48, Michael S. Tsirkin wrote:
>>>>>>>>>>> On Thu, Jul 16, 2026 at 05:59:05PM +0200, David Hildenbrand (Arm) wrote:
>>>>>>>>>>>>> Or do we just always trust virtio mem devices explicitly?
>>>>>>>>>>>> It's hard for me to understand where we draw the line, really.
>>>>>>>>>>>>
>>>>>>>>>>>> But maybe MST can clarify what we care about in virtio world where the
>>>>>>>>>>>> hypervisor is fully in charge of the device,
>>>>>>>>>>> Generally:
>>>>>>>>>>> - The guest is expected to whitelist drivers (most drivers have not
>>>>>>>>>>> been audited).
>>>>>>>>>> But even if you audited your driver, who makes sure that we consider all ways
>>>>>>>>>> where the device could mess with us?
>>>>>>>>> A lot of this is up to a correct setup. For example, make sure all
>>>>>>>>> filesystems are encrypted and refuse to mount unencrypted ones.
>>>>>>>>>
>>>>>>>>>> Something feels off here.
>>>>>>>>>>
>>>>>>>>>> Handling selected out-of-spec scenarios like this feels like a band-aid. Happy
>>>>>>>>>> to be corrected.
>>>>>>>>> Well Documentation/security/snp-tdx-threat-model.rst puts it like this:
>>>>>>>>> It is important to note
>>>>>>>>> that this doesn’t imply that the host or VMM are intentionally
>>>>>>>>> malicious, but that there exists a security value in having a small CoCo
>>>>>>>>> VM TCB.
>>>>>>>>>
>>>>>>>>> and
>>>>>>>>>
>>>>>>>>> While traditionally the host has unlimited access to guest data and can
>>>>>>>>> leverage this access to attack the guest, the CoCo systems mitigate such
>>>>>>>>> attacks by adding security features like guest data confidentiality and
>>>>>>>>> integrity protection.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> now, when we are talking about "mitigation" it is indeed becoming a bit
>>>>>>>>> murky.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> For me, a rule of thumb I came up with is that if the validation happens
>>>>>>>>> to also be helful for users e.g. to work around buggy devices,
>>>>>>>>> or maybe because we feel failing gracefully is nice because this
>>>>>>>>> will allow to later make use of this config and old drivers will
>>>>>>>>> fail but at least not panic, then it is good to include.
>>>>>>>> Why not do what USB does? Don't trust the device until AFTER probe()
>>>>>>>> succeeds? All of the needed checking should happen before then, as that
>>>>>>>> is a "slow path" so lots of validation and the like can happen at that
>>>>>>>> point.
>>>>>>>>
>>>>>>>> After that, during the normal data paths, after the driver is bound,
>>>>>>>> trust it all you want as attempting to validate every single packet is
>>>>>>>> just going to be impossible.
>>>>>>>>
>>>>>>>> thanks,
>>>>>>>>
>>>>>>>> greg k-h
>>>>>>> People do expect that data path validation at this point.
>>>>>> Ok, so you want this patch :)
>>>>>>
>>>>>> And more, as you need to treat everything from the host as "untrusted",
>>>>>> and it must be "verified".
>>>>> Well. First it's not me) Second it's only specific configurations -
>>>>> for example there's no short term plan to validate filesystem code, people
>>>>> are expected to rely on encryption. The reasons have more to do
>>>>> with the available manpower than anything else.
>>>> Sure, but again, for subsystems, you have to define your threat model as
>>>> the LLMs are churning against the code base and coming up with lots of
>>>> crazy ideas if a device should or should not be trusted and spitting out
>>>> patches and reports like the ones that are in the first few patches of
>>>> this series.
>>>>
>>>> So please, pick a model, let's document it, and go with that. I am
>>>> getting directly conflicting responses here.
>>>>
>>>> thanks,
>>>>
>>>> greg k-h
>>> Supposed to be this one:
>>> Documentation/security/snp-tdx-threat-model.rst
>>>
>>> what is missing?
>> A policy decision that needs to be made. All that document does is
>> describe a bunch of different "threats" yet does not decide what to do
>> about them at all from what I can tell.
> That would be this section I think:
>
> The **Linux kernel CoCo VM security objectives** can be summarized as follows:
>
> it does, indeed, not go into detail about how to interact, safely,
> with untrusted entities. Does it really need to be spelled out?
>
>> And that's just for one subset of the CoC world, right? Is that
>> something that all virtio drivers need/want to care about?
> What is missing, and what you seem to be asking for, is an opinionated
> stance on which drivers we care about in this world?
> True.
> coco guys tried to annotate drivers at some point to do exactly that.
> this was rejected upstream from the position that this is not
> different from handling buggy hardware, and just to fix all drivers.
> so it's up to users, and I guess for virtio the answer is yes
> with some exceptions because we don't have a better answer right now.
>
>> So I don't see a real answer to the "does Linux trust the host to give
>> you good data or not" question in that file, am I missing it?
>>
>> thanks,
>>
>> greg k-h
> This? Note the last sentence.
>
> The **Linux CoCo VM attack surface** is any interface exposed from a CoCo
> guest Linux kernel towards an untrusted host that is not covered by the
> CoCo technology SW/HW protection. This includes any possible
> side-channels, as well as transient execution side channels. Examples of
> explicit (not side-channel) interfaces include accesses to port I/O, MMIO
> and DMA interfaces, access to PCI configuration space, VMM-specific
> hypercalls (towards Host-side VMM), access to shared memory pages,
> interrupts allowed to be injected into the guest kernel by the host, as
> well as CoCo technology-specific hypercalls, if present. Additionally, the
> host in a CoCo system typically controls the process of creating a CoCo
> guest: it has a method to load into a guest the firmware and bootloader
> images, the kernel image together with the kernel command line. All of this
> data should also be considered untrusted until its integrity and
> authenticity is established via attestation.
I'm glad you're finding this document helpful, it took us massive
back-and-forth to get somewhere everyone was happy.
Here's my 2 cents on this debate, if I may. I think defensive programming
is always a positive, and we don't just say, "the spec disallows it".
Historically, one of the biggest criticisms of coco, especially around
device hardening, was that there were too many values that a
malicious/buggy device could misreport, making it a losing battle. That is
no longer the case with LLMs, and we have the advantage (and challenge) of
open-source dev, which allows us to receive many of these fixes "for free".
If others want to burn their tokens, let them :)
Thanks,
Carlos
>
>
>
^ permalink raw reply
* Re: [PATCH] vhost-scsi: Prevent OOM from invalid protection SGL count
From: Jia Jia @ 2026-07-18 4:14 UTC (permalink / raw)
To: bvel, rtik, efwo, kodw, michael.christie
Cc: slf, mst, jasowang, pbonzini, stefanha, eperezma, virtualization,
kvm, linux-kernel
In-Reply-To: <8f5b5350-0be9-4b79-a6c9-069d418dee30@oracle.com>
Before sending my patch, I did a duplicate check against the upstream
linux-next tree, upstream history, publicly searchable mailing-list archives,
and Patchwork. I searched using the vhost-scsi and scatterlist function names,
T10-PI, invalid protection SGL counts, and the OOM/resource-exhaustion
symptoms. I did not find Linfeng's earlier patch.
Could you please point me to Linfeng's patch, or confirm what the additional
check covers? In particular, does it also validate:
prot_bytes <= exp_data_len
before subtracting prot_bytes from exp_data_len and entering the SGL
mapping path? I had identified this as a related PI length-mismatch path, but
did not include it in the patch I sent.
With a one-byte combined payload and
pi_bytesout=2, KGDB stopped at the following call stack:
#0 sg_alloc_table_chained(
table=0xffff88810bbc5108,
nents=0,
first_chunk=0xffff888110050000,
nents_first_chunk=2048)
at lib/sg_pool.c:117
#1 vhost_scsi_handle_vq+2295
#2 vhost worker task
#3 srso_alias_return_thunk
The stop was at the `BUG_ON(!nents)` instruction in
`sg_alloc_table_chained()`, and the kernel log recorded `kernel BUG at
lib/sg_pool.c:117!`. This confirms that the PI length check is needed before
the iterator adjustment and SGL mapping path.
If Linfeng's patch covers both the invalid protection SGL count and this PI
length check, I will not submit duplicate work.
Regarding the AI question: AI assistance was used during the source analysis
and test development. I also checked the relevant code path and the
host-side behavior.
^ permalink raw reply
* Re: [PATCH v2 1/4] virtio-mem: validate device-reported block size
From: Greg Kroah-Hartman @ 2026-07-18 5:29 UTC (permalink / raw)
To: Carlos Bilbao
Cc: Michael S. Tsirkin, David Hildenbrand (Arm), Hari Mishal,
Jason Wang, Xuan Zhuo, Eugenio Pérez, virtualization,
linux-kernel, elena.reshetova
In-Reply-To: <1fe328d1-edf9-4e72-a145-be74ede20e60@gmail.com>
On Fri, Jul 17, 2026 at 08:31:09PM -0700, Carlos Bilbao wrote:
> Historically, one of the biggest criticisms of coco, especially around
> device hardening, was that there were too many values that a
> malicious/buggy device could misreport, making it a losing battle. That is
> no longer the case with LLMs, and we have the advantage (and challenge) of
> open-source dev, which allows us to receive many of these fixes "for free".
> If others want to burn their tokens, let them :)
I have lots of tokens to burn :)
So along those lines, any suggestions on how best to fuzz these code
paths? Any workloads you all use for testing that I can take advantage
of?
thanks,
greg k-h
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox