[Xen-devel] Ill-advised use of xs_open flag 1UL<<2 by Debian

[Ian Jackson <ijackson@xxxxxxxxxxxxxxxxxxxxxx>]

tl;dr
  The Debian Xen packages have had a very bad patch which I propose to
  simply drop, with minor compatibility implications, unless someone
  can explain what it is for and why it is still needed, and/or has a
  better plan.



I have been going through delta queue in the Debian Xen package.
I found a commit (patch) describing itself only this way:
   xenstore/tools-xenstore-compatibility.diff
Yes, that was the whole of the description.

The patch
 - defines XS_OPEN_DOMAINONLY to 1UL<<2
 - arranges that when this flag is passed, xs_open will never try
   the socket, only the xenstore kernel device
 - always passes that flag in the xenstore utilities.

Digging into snapshot.d.o I found this changelog entry in 4.1.0-2,
  * Workaround incompatibility with xenstored of Xen 4.0.
which I can tell by diffing 4.1.0-1 and 4.1.0-2 must refer to this.
I was not able to discover what the incompatibility was.  There
did not seem to be any related archived bugs, or commentary, or
anything.  It may be related to one of these
  d31038383192 xenstore: new XS_OPEN_SOCKETONLY flag ...
  999241f7aa45 Adds an open xenstore connection function ...
(commitids are from usptream xen.git)

In any case, if this patch previously served any purpose it doesn't
seem to any more.  xenstore socket connections work just fine in dom0,
and attempting them in domUs does no harm.  I built salsa/master with
this patch reverted and both an HVM and PV domU had no trouble
accessing xenstore.

This patch was extremely ill-advised because it stole a bit from the
xs_open flag bitmap without coordination with upstream.

In Xen 4.3, upstream instroduced XS_UNWATCH_FILTER with value 1UL<<2.

In April 2011, the Debian Xen packaging was updated to Xen 4.4; this
included rebasing the patch queue.  At this time, this patch would
have produced a conflict - both textual (context for the Debian patch
changed due to the upstream addition) and semantic (same value used
twice).  Unfortunately the conflict was resolved textually, and the
semantic conflict was ignored.  Debian's 4.4 simply took both
additions without noticing that the value 1UL<<2 was used for two
different purposes.

This simple #define style of flag allocation does not provide any
programmatic way of detecting this kind of situation.  But perhaps a
contributing factor to this further error was that, upstream, while
XS_UNWATCH_FILTER is a flag to xs_open, the name chosen did not match
XS_OPEN_*.

Since then, the Debian Xen packages have conflated these two flags:

Attempts by any application or program linked against libxenstore to
use XS_UNWATCH_FILTER will have had as a side effect that they would
use only the xenstore domain connection (via the kernel device) and
not the socket.  I think that is almost always almost entirely
harmless.

The converse is that any programs trying to use XS_OPEN_DOMAINONLY
would get the watch filtering effect.  In a complex application the
watch filtering effect could cause lost xenstore watch events.
Luckily I think the only thing that is likely to pass this flag is the
xenstore utilities in the Debian Xen packages and those are not
complicated enough to be affected.

I propose to resolve this situation in Debian by summarily dropping
this patch in the next Debian stable release.  (We will leave well
enough alone in the previous two Debian releases.)  I think the only
user of XS_OPEN_DOMAINONLY is probably in src:xen itself.

I also observe that, upstream, these flag values are not enclosed in
( ) as they clearly should be.  I will send a patch upstream to fix
that.


If anyone has any information about this situation, particularly
information or analysis which might suggest that just dropping the
patch would be the wrong thing to do, please let us (me, and
pkg-xen-deve) know.

Thanks,
Ian.

-- 
Ian Jackson <ijackson@xxxxxxxxxxxxxxxxxxxxxx>   These opinions are my own.

If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel