Re: control/shutdown node removed on suspend?

[Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>]

On Wed, May 04, 2022 at 05:42:51PM +0100, Durrant, Paul wrote:
> On 04/05/2022 16:33, Marek Marczykowski-Górecki wrote:
> > Hi,
> > 
> > I've just got an interesting bug report - it seems Windows PV drivers (or
> > something else?) remove control/shutdown xenstore node on suspend,
> > instead of just writing empty string there (to ack the request).
> > This makes subsequent suspend to fail, because toolstack does not adjust
> > permissions of that node when writing to it (assuming it was set
> > correctly during domain construction), which means domU has no longer
> > write access to it and cannot acknowledge the request.
> > 
> > TBH, I'm not sure where exactly this happens, could be also outside of
> > PV drivers. But the issue doesn't apply for Linux VMs - that's why I
> > think it is related to Windows PV drivers.
> > https://xenbits.xen.org/gitweb/?p=pvdrivers/win/xeniface.git;a=blob;f=src/xenagent/service.cpp;h=9bc5c20767454eaeae4760308d7655580a08e4cb;hb=5c6adcaa423c465de279158c4cc42500627c20a0
> > seems to use StoreWrite("control/shutdown", ""), which sounds correct
> > (if it really does what it suggests to do). But that's just cursory look
> > at the code, I'm not sure if that's even the correct file.
> > 
> > The system is using official build of 8.2.2 drivers.
> > 
> > Original bug report can be found here:
> > https://github.com/QubesOS/qubes-issues/issues/7404#issuecomment-1117231111
> > (see subsequent comments for details)
> > 
> > Any idea?
> > 
> 
> The commit you are missing, I believe, is:
> 
> commit 4c1d9b0a57a349d9d3ae8c8af2519618c73c40b4
> Author: Paul Durrant <paul.durrant@xxxxxxxxxx>
> Date:   Tue Apr 10 17:21:38 2018 +0100
> 
>     Fix control/shutdown acknowledgement
> 
>     The code in FdoSuspend() currently acknowledges a suspend request by
>     removing the control/shutdown node from xenstore. This is not correct.
> 
>     The documentation [1] states that a request should be acknowledged by
>     writing the empty string. This patch makes the code behave correctly.
> 
>     [1] 
> xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=docs/misc/xenstore-paths.markdown;hb=HEAD#l413
> 
>     Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> 
> This is what git says:
> 
> git tag --contains 4c1d9b0a57a349d9d3ae8c8af2519618c73c40b4
> 9.0.0-rc1
> 
> ... so it was not back-ported to 8.x.

Looks like it, thanks!

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

Attachment: signature.asc
Description: PGP signature