talex5 / wayland-proxy-virtwl

Allow guest VMs to open windows on the host
Apache License 2.0
114 stars 12 forks source link

Protect against clients truncating shared memory buffers #92

Open DemiMarie opened 3 months ago

DemiMarie commented 3 months ago

Clients can truncate shared memory buffers, causing SIGBUS to be raised. This must be handled in C (not OCaml).

The best way I can think of to solve this is to check if the signal handler interrupted the copying code, and if it did, longjmp out of the handler. This means that the copying function must be async signal safe, but I believe memcpy always is: compilers are allowed to insert calls to it wherever they want, so if it was not async signal safe it would be impossible to write a correct signal handler.

talex5 commented 3 months ago

It should be possible to prevent that with e.g. fcntl(fd, F_ADD_SEALS, F_SEAL_SHRINK).

DemiMarie commented 3 months ago

F_ADD_SEALS only works on memfds, and Wayland clients are not required to use memfds.

talex5 commented 3 months ago

I guess a client could use a regular file, though it seems a bit unlikely. Does it happen in real applications?

But if you're running your proxy on the host then it doesn't matter as the application in the VM presumably can't resize the host memory (and we're not copying then anyway). And if the proxy is running inside the VM then we're not particularly trying to prevent the application from crashing it.

DemiMarie commented 3 months ago

I guess a client could use a regular file, though it seems a bit unlikely. Does it happen in real applications?

It’s not as unlikely as one might expect, because memfds are Linux-specific and the Wayland protocol is not.

But if you're running your proxy on the host then it doesn't matter as the application in the VM presumably can't resize the host memory (and we're not copying then anyway). And if the proxy is running inside the VM then we're not particularly trying to prevent the application from crashing it.

The wl_shm implementations in other Wayland compositor libraries do protect against clients that truncate buffers.