search

RustAudio / baseview

low-level window system interface for audio plugin UIs
Apache License 2.0
259 stars 54 forks source link

Fix GL X11 error handling #181

Closed prokopyl closed 3 months ago

prokopyl commented 3 months ago

This PR fixes a soundness issue inside the XErrorHandler utility function, which could use an xlib Display pointer that had no guarantee to be still valid.

This could happen because the underlying XErrorEvent was stored directly inside the returned error type, and the Display pointer it contained was only called to extract the error message during in Debug implementation, which could happen long after the associated Display had been destroyed.

This PR fixes this by extracting the error message upfront and storing it as a string as soon as the error happens. This PR also fixes the handle method that was not properly marked unsafe.

prokopyl commented 3 months ago

I don't 100% love the idea of eagerly fetching the error text, but I guess it's preferable to alternatives like having the error enum keep the display alive with reference counting.

Yeah it didn't feel right to me either initially, but the X docs support that calling XGetErrorText within the error handler is safe (it doesn't do any server requests). Plus it's just formatting a few strings which is negligible performance-wise, even more so considering it's only for the cold (error) path on the 3 functions we're calling.