Fix use-after-unmap segfault: surface must hold mapping ref
The eager-unmap logic (0c6c366) tracked mapping references per buffer but
not per surface. When a surface attached a virtwl buffer, it stored Cstruct
views into the mapping's bigarrays. If the buffer (and pool) were then
destroyed, the mapping was unmapped while the surface still held dangling
Cstruct references. The next commit would Cstruct.blit through the freed
bigarray data pointer (NULL post-unmap), segfaulting in libc memcpy.
Fix: the surface now participates in mapping ref counting. On attach it
increments the mapping ref via a closure on the virtwl_buffer; on re-attach,
null-attach, or surface destroy it decrements the old one. This keeps the
mapping alive as long as any surface references its memory.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
0c6c3666b3
commit
5a6339b134
3 changed files with 21 additions and 360 deletions
|
|
@ -1,51 +0,0 @@
|
|||
# Force Server-Side Decorations
|
||||
|
||||
## Problem
|
||||
|
||||
This proxy is used in a Qubes-like VM isolation system where Wayland applications in VMs display on a host compositor. The proxy tags window titles with VM names to identify which VM owns each window.
|
||||
|
||||
Client-side decorations (CSD) break this security model: an app in one VM could draw a fake title bar impersonating another VM. Server-side decorations must always be used so the trusted title bar (drawn by the host compositor) cannot be spoofed.
|
||||
|
||||
## Solution
|
||||
|
||||
Force server-side decorations unconditionally for all windows.
|
||||
|
||||
### Host Side
|
||||
|
||||
When creating an `xdg_toplevel` on the host:
|
||||
|
||||
1. Create `xdg_toplevel_decoration_v1` for that toplevel
|
||||
2. Call `set_mode(server_side)` to request SSD
|
||||
3. Handle the `configure` event from the host
|
||||
|
||||
This happens for every toplevel, regardless of whether the client uses the decoration protocol.
|
||||
|
||||
If the host compositor doesn't support `zxdg_decoration_manager_v1`, log a warning and continue. Decorations will depend on compositor defaults.
|
||||
|
||||
### Client Side
|
||||
|
||||
When a client uses the decoration protocol:
|
||||
|
||||
1. Client creates `zxdg_toplevel_decoration_v1` - proxy creates corresponding object
|
||||
2. Client calls `set_mode(...)` or `unset_mode()` - proxy ignores the preference
|
||||
3. Proxy sends `configure(server_side)` back to client
|
||||
4. Compliant clients will not draw their own decorations
|
||||
|
||||
Clients that don't use the decoration protocol may draw their own title bar, but it appears inside the trusted frame and cannot cover the real title bar.
|
||||
|
||||
## Security Properties
|
||||
|
||||
- The host compositor draws decorations showing the VM-tagged title
|
||||
- Clients cannot draw outside their allocated surface area (Wayland enforces this)
|
||||
- Even if a client draws a fake title bar in its content, it's inside the trusted frame
|
||||
|
||||
## Files to Modify
|
||||
|
||||
- `relay.ml` - Create decoration object on host side, intercept client decoration requests
|
||||
- `host.ml` - Track whether host supports decoration manager, log warning if not
|
||||
|
||||
## Testing
|
||||
|
||||
- GTK app (defaults to CSD) should get SSD
|
||||
- Qt app should get SSD
|
||||
- Verify warning logged when host lacks xdg-decoration support
|
||||
|
|
@ -1,306 +0,0 @@
|
|||
# Force Server-Side Decorations Implementation Plan
|
||||
|
||||
> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task.
|
||||
|
||||
**Goal:** Force server-side decorations for all windows to prevent client apps from spoofing VM identity via fake title bars.
|
||||
|
||||
**Architecture:** When a toplevel is created, the proxy automatically creates a decoration object on the host and requests server-side mode. When clients use the decoration protocol, the proxy intercepts their requests and always responds with server_side mode.
|
||||
|
||||
**Tech Stack:** OCaml, ocaml-wayland library, xdg-decoration-unstable-v1 protocol
|
||||
|
||||
---
|
||||
|
||||
## Task 1: Pass host registry to make_xdg_wm_base
|
||||
|
||||
The `make_toplevel` function needs access to the host registry to create decorations. We need to thread the registry through from `make_xdg_wm_base` to `make_xdg_surface` to `make_toplevel`.
|
||||
|
||||
**Files:**
|
||||
- Modify: `relay.ml:854-878` (make_xdg_wm_base)
|
||||
- Modify: `relay.ml:813-833` (make_xdg_surface)
|
||||
- Modify: `relay.ml:784-811` (make_toplevel)
|
||||
|
||||
**Step 1: Update make_toplevel signature to accept host registry**
|
||||
|
||||
In `relay.ml`, change `make_toplevel` to accept the host registry:
|
||||
|
||||
```ocaml
|
||||
let make_toplevel ~registry ~tag ~host_toplevel c =
|
||||
```
|
||||
|
||||
**Step 2: Update make_xdg_surface to pass registry to make_toplevel**
|
||||
|
||||
Change `make_xdg_surface` signature and the call to `make_toplevel`:
|
||||
|
||||
```ocaml
|
||||
let make_xdg_surface ~registry ~tag ~host_xdg_surface c =
|
||||
```
|
||||
|
||||
And update the `on_get_toplevel` method:
|
||||
|
||||
```ocaml
|
||||
method on_get_toplevel _ = make_toplevel ~registry ~tag ~host_toplevel:(H.Xdg_surface.get_toplevel h)
|
||||
```
|
||||
|
||||
**Step 3: Update make_xdg_wm_base to pass registry to make_xdg_surface**
|
||||
|
||||
Change `make_xdg_wm_base` signature:
|
||||
|
||||
```ocaml
|
||||
let make_xdg_wm_base ~registry ~xwayland ~tag bind proxy =
|
||||
```
|
||||
|
||||
And update the call to `make_xdg_surface`:
|
||||
|
||||
```ocaml
|
||||
make_xdg_surface ~registry ~tag ~host_xdg_surface xdg_surface
|
||||
```
|
||||
|
||||
**Step 4: Update on_bind to pass registry to make_xdg_wm_base**
|
||||
|
||||
In `make_registry`, update the call:
|
||||
|
||||
```ocaml
|
||||
| Xdg_wm_base.T -> make_xdg_wm_base ~registry:t.host.registry ~xwayland ~tag:t.config.tag bind proxy
|
||||
```
|
||||
|
||||
**Step 5: Build to verify no compilation errors**
|
||||
|
||||
Run: `nix build '.?submodules=1'`
|
||||
Expected: Build succeeds (no functional change yet)
|
||||
|
||||
**Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add relay.ml
|
||||
git commit -m "Thread host registry through to make_toplevel
|
||||
|
||||
Preparation for creating decoration objects when toplevels are created.
|
||||
|
||||
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 2: Create host-side decoration when toplevel is created
|
||||
|
||||
When a toplevel is created on the host, also create a decoration object and request server-side mode.
|
||||
|
||||
**Files:**
|
||||
- Modify: `relay.ml:784-811` (make_toplevel)
|
||||
|
||||
**Step 1: Add module aliases at the top of relay.ml**
|
||||
|
||||
Add after the existing module imports (around line 1-10):
|
||||
|
||||
```ocaml
|
||||
module Xdg_decor_mgr = H.Zxdg_decoration_manager_v1
|
||||
module Xdg_decoration = H.Zxdg_toplevel_decoration_v1
|
||||
```
|
||||
|
||||
**Step 2: Update make_toplevel to create decoration on host**
|
||||
|
||||
Replace `make_toplevel` with:
|
||||
|
||||
```ocaml
|
||||
let make_toplevel ~registry ~tag ~host_toplevel c =
|
||||
let h = host_toplevel @@ object
|
||||
inherit [_] H.Xdg_toplevel.v1
|
||||
method on_close _ = C.Xdg_toplevel.close c
|
||||
method on_configure _ = C.Xdg_toplevel.configure c
|
||||
method on_configure_bounds _ = C.Xdg_toplevel.configure_bounds c
|
||||
method on_wm_capabilities _ = C.Xdg_toplevel.wm_capabilities c
|
||||
end
|
||||
in
|
||||
(* Create decoration on host to force server-side decorations *)
|
||||
begin match Registry.get registry Xdg_decor_mgr.interface with
|
||||
| [] ->
|
||||
Log.warn (fun f -> f "Host doesn't support %s - server-side decorations may not be enforced" Xdg_decor_mgr.interface)
|
||||
| { Registry.name; version } :: _ ->
|
||||
let decor_mgr = H.Wl_registry.bind (Registry.wl_registry registry) ~name (object
|
||||
inherit [_] Xdg_decor_mgr.v1
|
||||
end, min 1l version)
|
||||
in
|
||||
let _decor = Xdg_decor_mgr.get_toplevel_decoration decor_mgr ~toplevel:h @@ object
|
||||
inherit [_] Xdg_decoration.v1
|
||||
method on_configure _ ~mode:_ = ()
|
||||
end
|
||||
in
|
||||
Xdg_decoration.set_mode _decor ~mode:Xdg_decoration.Mode.Server_side
|
||||
end;
|
||||
let user_data = client_data (Toplevel h) in
|
||||
Proxy.Handler.attach c @@ object
|
||||
inherit [_] C.Xdg_toplevel.v1
|
||||
method! user_data = user_data
|
||||
method on_destroy = delete_with H.Xdg_toplevel.destroy h
|
||||
method on_move _ ~seat = H.Xdg_toplevel.move h ~seat:(to_host seat)
|
||||
method on_resize _ ~seat = H.Xdg_toplevel.resize h ~seat:(to_host seat)
|
||||
method on_set_app_id _ = H.Xdg_toplevel.set_app_id h
|
||||
method on_set_fullscreen _ ~output = H.Xdg_toplevel.set_fullscreen h ~output:(Option.map to_host output)
|
||||
method on_set_max_size _ = H.Xdg_toplevel.set_max_size h
|
||||
method on_set_maximized _ = H.Xdg_toplevel.set_maximized h
|
||||
method on_set_min_size _ = H.Xdg_toplevel.set_min_size h
|
||||
method on_set_minimized _ = H.Xdg_toplevel.set_minimized h
|
||||
method on_set_parent _ ~parent = H.Xdg_toplevel.set_parent h ~parent:(Option.map to_host parent)
|
||||
method on_set_title _ ~title = H.Xdg_toplevel.set_title h ~title:(tag ^ title)
|
||||
method on_show_window_menu _ ~seat = H.Xdg_toplevel.show_window_menu h ~seat:(to_host seat)
|
||||
method on_unset_fullscreen _ = H.Xdg_toplevel.unset_fullscreen h
|
||||
method on_unset_maximized _ = H.Xdg_toplevel.unset_maximized h
|
||||
end
|
||||
```
|
||||
|
||||
**Step 3: Build to verify compilation**
|
||||
|
||||
Run: `nix build '.?submodules=1'`
|
||||
Expected: Build succeeds
|
||||
|
||||
**Step 4: Commit**
|
||||
|
||||
```bash
|
||||
git add relay.ml
|
||||
git commit -m "Create host-side decoration requesting server-side mode
|
||||
|
||||
When a toplevel is created, automatically create a decoration object on
|
||||
the host and request server_side mode. This ensures the host compositor
|
||||
draws the title bar showing the VM-tagged window title.
|
||||
|
||||
Logs a warning if host doesn't support xdg-decoration protocol.
|
||||
|
||||
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 3: Force server_side response for client decoration requests
|
||||
|
||||
When clients use the decoration protocol, always respond with `server_side` regardless of what they request.
|
||||
|
||||
**Files:**
|
||||
- Modify: `relay.ml:950-974` (make_xdg_decoration and make_xdg_decoration_manager)
|
||||
|
||||
**Step 1: Update make_xdg_decoration to always send server_side to client**
|
||||
|
||||
Replace `make_xdg_decoration` with:
|
||||
|
||||
```ocaml
|
||||
let make_xdg_decoration ~host_decoration c =
|
||||
let h = host_decoration @@ object
|
||||
inherit [_] H.Zxdg_toplevel_decoration_v1.v1
|
||||
method on_configure _ ~mode:_ =
|
||||
(* Always tell client to use server-side decorations, regardless of host response *)
|
||||
C.Zxdg_toplevel_decoration_v1.configure c ~mode:C.Zxdg_toplevel_decoration_v1.Mode.Server_side
|
||||
end
|
||||
in
|
||||
(* Request server-side from host *)
|
||||
H.Zxdg_toplevel_decoration_v1.set_mode h ~mode:H.Zxdg_toplevel_decoration_v1.Mode.Server_side;
|
||||
Proxy.Handler.attach c @@ object
|
||||
inherit [_] C.Zxdg_toplevel_decoration_v1.v1
|
||||
method on_destroy = delete_with H.Zxdg_toplevel_decoration_v1.destroy h
|
||||
method on_set_mode _ ~mode:_ =
|
||||
(* Ignore client preference, always request server-side from host *)
|
||||
H.Zxdg_toplevel_decoration_v1.set_mode h ~mode:H.Zxdg_toplevel_decoration_v1.Mode.Server_side
|
||||
method on_unset_mode _ =
|
||||
(* Even on unset, request server-side *)
|
||||
H.Zxdg_toplevel_decoration_v1.set_mode h ~mode:H.Zxdg_toplevel_decoration_v1.Mode.Server_side
|
||||
end
|
||||
```
|
||||
|
||||
**Step 2: Build to verify compilation**
|
||||
|
||||
Run: `nix build '.?submodules=1'`
|
||||
Expected: Build succeeds
|
||||
|
||||
**Step 3: Commit**
|
||||
|
||||
```bash
|
||||
git add relay.ml
|
||||
git commit -m "Force server_side response for client decoration requests
|
||||
|
||||
When clients use the xdg-decoration protocol, always tell them to use
|
||||
server_side mode regardless of their preference. This prevents apps from
|
||||
opting into client-side decorations which could be used to spoof VM
|
||||
identity.
|
||||
|
||||
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 4: Apply same treatment to KDE decoration protocol
|
||||
|
||||
The KDE server decoration protocol is another way clients can request decoration modes. Apply the same forcing logic.
|
||||
|
||||
**Files:**
|
||||
- Modify: `relay.ml:925-948` (make_kde_decoration and make_kde_decoration_manager)
|
||||
|
||||
**Step 1: Update make_kde_decoration to force server-side**
|
||||
|
||||
Replace `make_kde_decoration` with:
|
||||
|
||||
```ocaml
|
||||
let make_kde_decoration ~host_decoration c =
|
||||
let h = host_decoration @@ object
|
||||
inherit [_] H.Org_kde_kwin_server_decoration.v1
|
||||
method on_mode _ ~mode:_ =
|
||||
(* Always tell client to use server-side decorations *)
|
||||
C.Org_kde_kwin_server_decoration.mode c ~mode:C.Org_kde_kwin_server_decoration.Mode.Server
|
||||
end
|
||||
in
|
||||
(* Request server-side from host *)
|
||||
H.Org_kde_kwin_server_decoration.request_mode h ~mode:H.Org_kde_kwin_server_decoration.Mode.Server;
|
||||
Proxy.Handler.attach c @@ object
|
||||
inherit [_] C.Org_kde_kwin_server_decoration.v1
|
||||
method on_release = delete_with H.Org_kde_kwin_server_decoration.release h
|
||||
method on_request_mode _ ~mode:_ =
|
||||
(* Ignore client preference, always request server-side from host *)
|
||||
H.Org_kde_kwin_server_decoration.request_mode h ~mode:H.Org_kde_kwin_server_decoration.Mode.Server
|
||||
end
|
||||
```
|
||||
|
||||
**Step 2: Build to verify compilation**
|
||||
|
||||
Run: `nix build '.?submodules=1'`
|
||||
Expected: Build succeeds
|
||||
|
||||
**Step 3: Commit**
|
||||
|
||||
```bash
|
||||
git add relay.ml
|
||||
git commit -m "Force server-side in KDE decoration protocol too
|
||||
|
||||
Apply the same server-side decoration forcing to the KDE server
|
||||
decoration protocol for consistency.
|
||||
|
||||
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 5: Test manually with a real application
|
||||
|
||||
**Files:**
|
||||
- None (manual testing)
|
||||
|
||||
**Step 1: Build the proxy**
|
||||
|
||||
Run: `nix build '.?submodules=1'`
|
||||
|
||||
**Step 2: Test with a GTK application**
|
||||
|
||||
GTK apps default to client-side decorations. Run the proxy and launch a GTK app (e.g., `gnome-calculator` or `gedit`) to verify it gets server-side decorations from KDE.
|
||||
|
||||
Expected: Window should have KDE-drawn title bar, not GTK's client-side decorations.
|
||||
|
||||
**Step 3: Verify title tag appears**
|
||||
|
||||
The title bar should show the VM tag prefix (if `--tag` is used).
|
||||
|
||||
---
|
||||
|
||||
## Summary
|
||||
|
||||
After completing all tasks:
|
||||
1. All toplevels automatically get server-side decorations requested on the host
|
||||
2. Clients using xdg-decoration protocol are told to use server_side
|
||||
3. Clients using KDE decoration protocol are told to use server_side
|
||||
4. Host compositor (KDE) draws the trusted title bar showing VM identity
|
||||
5. Warning logged if host doesn't support decoration protocols
|
||||
24
relay.ml
24
relay.ml
|
|
@ -104,6 +104,8 @@ module CD = struct
|
|||
host_buffer : 'v H.Wl_buffer.t;
|
||||
host_memory : Cstruct.t;
|
||||
client_memory : Cstruct.t;
|
||||
mapping_incr : unit -> unit;
|
||||
mapping_decr : unit -> unit;
|
||||
}
|
||||
|
||||
type 'v buffer = [
|
||||
|
|
@ -121,6 +123,7 @@ module CD = struct
|
|||
mutable state : surface_state;
|
||||
mutable host_memory : Cstruct.t;
|
||||
mutable client_memory : Cstruct.t;
|
||||
mutable release_mapping : unit -> unit;
|
||||
}
|
||||
|
||||
(* Decoration info is separate so it can be retrieved without existential type issues *)
|
||||
|
|
@ -401,7 +404,9 @@ end = struct
|
|||
method on_release _ = C.Wl_buffer.release buffer
|
||||
end
|
||||
in
|
||||
{ CD.host_memory; client_memory; host_buffer }
|
||||
{ CD.host_memory; client_memory; host_buffer;
|
||||
mapping_incr = (fun () -> mapping.ref_count <- mapping.ref_count + 1);
|
||||
mapping_decr = (fun () -> mapping_dec_ref mapping) }
|
||||
)
|
||||
in
|
||||
let on_destroy = lazy (
|
||||
|
|
@ -449,7 +454,8 @@ let make_surface ~xwayland ~host_surface c =
|
|||
let h = Proxy.cast_version h in
|
||||
let data =
|
||||
let state = if xwayland = None then CD.Ready else Unconfigured (Queue.create ()) in
|
||||
{ CD.host_surface = h; host_memory = Cstruct.empty; client_memory = Cstruct.empty; state }
|
||||
{ CD.host_surface = h; host_memory = Cstruct.empty; client_memory = Cstruct.empty; state;
|
||||
release_mapping = ignore }
|
||||
in
|
||||
let when_configured fn =
|
||||
match data.state with
|
||||
|
|
@ -470,15 +476,25 @@ let make_surface ~xwayland ~host_surface c =
|
|||
let Client_data (Buffer buffer) = user_data buffer in
|
||||
let host_buffer =
|
||||
match buffer with
|
||||
| `Direct host_buffer -> host_buffer
|
||||
| `Direct host_buffer ->
|
||||
data.release_mapping ();
|
||||
data.release_mapping <- ignore;
|
||||
data.host_memory <- Cstruct.empty;
|
||||
data.client_memory <- Cstruct.empty;
|
||||
host_buffer
|
||||
| `Virtwl buffer ->
|
||||
let buffer = Shm.map_buffer buffer in
|
||||
data.release_mapping ();
|
||||
buffer.mapping_incr ();
|
||||
data.release_mapping <- buffer.mapping_decr;
|
||||
data.host_memory <- buffer.host_memory;
|
||||
data.client_memory <- buffer.client_memory;
|
||||
buffer.host_buffer
|
||||
in
|
||||
H.Wl_surface.attach h ~buffer:(Some host_buffer) ~x ~y
|
||||
| _ ->
|
||||
data.release_mapping ();
|
||||
data.release_mapping <- ignore;
|
||||
data.host_memory <- Cstruct.empty;
|
||||
data.client_memory <- Cstruct.empty;
|
||||
H.Wl_surface.attach h ~buffer:None ~x ~y
|
||||
|
|
@ -500,6 +516,8 @@ let make_surface ~xwayland ~host_surface c =
|
|||
H.Wl_surface.damage_buffer h ~x ~y ~width ~height
|
||||
|
||||
method on_destroy =
|
||||
data.release_mapping ();
|
||||
data.release_mapping <- ignore;
|
||||
data.state <- Destroyed;
|
||||
xwayland |> Option.iter (fun (x:xwayland_hooks) -> x#on_destroy_surface h);
|
||||
delete_with H.Wl_surface.destroy h
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue