Subject: Re: [PATCH iwl-next v4 09/12] ice: Save and load TX Queue head

From: Jason Gunthorpe <[email-masked]>
Date: Thu, 7 Dec 2023 10:48:07 -0400

On Thu, Dec 07, 2023 at 08:22:53AM +0000, Tian, Kevin wrote:
> > In virtual channel model, VF driver only send TX queue ring base and
> > length info to PF, while rest of the TX queue context are managed by PF.
> > TX queue length must be verified by PF during virtual channel message
> > processing. When PF uses dummy descriptors to advance TX head, it will
> > configure the TX ring base as the new address managed by PF itself. As a
> > result, all of the TX queue context is taken control of by PF and this
> > method won't generate any attacking vulnerability
> 
> So basically the key points are:
> 
> 1) TX queue head cannot be directly updated via VF mmio interface;
> 2) Using dummy descriptors to update TX queue head is possible but it
>     must be done in PF's context;
> 3) FW provides a way to keep TX queue head intact when moving
>     the TX queue ownership between VF and PF;
> 4) the TX queue context affected by the ownership change is largely
>     initialized by the PF driver already, except ring base/size coming from
>     virtual channel messages. This implies that a malicious guest VF driver
>     cannot attack this small window though the tx head restore is done
>     after all the VF state are restored;
> 5) and a missing point is that the temporary owner change doesn't
>     expose the TX queue to the software stack on top of the PF driver
>     otherwise that would be a severe issue.

This matches my impression of these patches. It is convoluted but the
explanation sounds find, and if Intel has done an internal security
review then I have no issue.

Jason