On 17/10/09 16:00, Leonid wrote:
* Has anyone hit this particular issue before? If so, what did they do?
* Does my plan above sound sane? Any suggestions?
* What's the proper way to set the system context (i.e. really run
glxMakeCurrent) without introducing inconsistencies in server state?
... [the] render spu should bind default window to all contexts if its
currently
bound window is being destroyed. I've commited the fix, so it should
be in the
ose soon. Could you please check if your issue is gone and post a
followup here.
I just finished coding a similar solution, I'll see how our changes
compare :)
In the course of doing so, I got to know the render SPU's "default
window," which it creates at startup and uses to perform real-libGL
MakeCurrents when the client hasn't sent us a WindowCreate yet, and the
client might send GL commands which are legal without a current context
(e.g. create/destroy-context) which on the server are executed using GL
commands which *do* require a current context -- like the previous
example with state_diff using glEnable and other state functions.
This looks like a minor problem -- a client could skip issuing a
WindowCreate and start sending rendering commands, and the render SPU
would map the default window. The default window is not a child of the
VBox guest display window, so there would just be a random top-level
window containing the guest's 3D content. It seems to me the guest
shouldn't be able to "escape" its bounds like this, even cosmetically --
and further, it means Chromium is accepting what would be an illegal
OpenGL program -- moving straight to glBegin(); glVertex... without a
glX/wgl/agl/cglCreateContext+MakeCurrent ought to result in an error.
A separate issue I found is that in the server's implementation of
WindowDestroy, it notes that "some apps destroy windows in a different
thread than they created it", and so spools through all the current
clients, killing like-numbered windows. It seems to me that there ought
to be a line between threads in one process and separate processes, as
otherwise we could have the following situation:
A guest could run a regular GL program (say, gears,) and a malicious
program. The malicious program could simply guess at GL's CrWindow ID
and send a WindowDestroy; the server would then regard that as an
out-of-thread destroy, and zap gears' window, which would then try to
draw illegally -- and in fact, hit the default zero-window, according to
your new fix. This seems to me as though it's allowing processes within
one VM to violate the isolation which their operating system is supposed
to provide. The same ID-guessing game could be played with contexts, and
possibly other resources like textures (I haven't investigated how
they're named by Cr yet).
I think a solution might be:
* Multithreading GL programs are spotted by the guest library, and their
commands muxed onto a single TCP / HGCM channel. (Looks like VBox is
using HGCM by default and TCP with a couple of undefs; I've hacked it
back to TCP for my purposes)
* * Obviously that would mean the server needs a concept of per-thread
concept, which it almost has already.
* Window, context and other vulnerable IDs are allocated per-connection,
not per-VM as at present.
* Therefore, it's not possible to make reference to out-of-process
resources.
The only problem here would be coordinating the
new-thread-starts-rendering process, but there's a bunch of code
detecting this situation in the Chromium libGL stub already -- the
server would just need minor alterations to introduce the extra
hierarchy in which it has a connection per client, which have a pool of
contexts and windows, and the clients have threads, and the threads have
a current-context and current-window like clients do now, with switching
happening the same way as now. Then the server would scan for other
threads using the same window / context -- but not other processes.
Chris
_______________________________________________
vbox-dev mailing list
[email protected]
http://vbox.innotek.de/mailman/listinfo/vbox-dev
