fix(core): keep destroyed objects as zombies until delete_id
SendDestructor replaced the object with nothing, so any event the compositor had already sent for it failed the connection with ErrUnknownObject. The object table now holds a zombie until wl_display.delete_id: in-flight events are validated against the original signature, their descriptors are consumed and closed, and no handler runs. delete_id removes the zombie and frees the ID for reuse.
- commit
- 82fabc3abec7e65159f395013eebe3788a6ce120
- author
- brice · · 2026-09-29 15:44 UTC
- parent
- 6cc258a
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +96 -3
M client.go +16 -2
| @@ -500,6 +500,9 @@ func (d *Display) dispatchFrame(f receivedFrame) error { | |||
| 500 | 500 | *ev = Event{} | |
| 501 | 501 | eventPool.Put(ev) | |
| 502 | 502 | }() | |
| 503 | + | if _, isZombie := obj.(*zombie); isZombie { | |
| 504 | + | return nil | |
| 505 | + | } | |
| 503 | 506 | if proxy, ok := obj.(Proxy); ok { | |
| 504 | 507 | proxy.Dispatch(ev) | |
| 505 | 508 | } else { | |
| @@ -542,8 +545,13 @@ func (d *Display) prepareFrame(f receivedFrame) (Object, *Event, uint8, error) { | |||
| 542 | 545 | if !ok { | |
| 543 | 546 | return nil, nil, 0, &ProtocolError{Kind: "unknown_object", Object: f.object, Opcode: f.opcode, Err: ErrUnknownObject} | |
| 544 | 547 | } | |
| 548 | + | sigObj := obj | |
| 549 | + | z, isZombie := obj.(*zombie) | |
| 550 | + | if isZombie { | |
| 551 | + | sigObj = z.object | |
| 552 | + | } | |
| 545 | 553 | sig, valid := "", false | |
| 546 | - | p, isGenerated := obj.(signatureProvider) | |
| 554 | + | p, isGenerated := sigObj.(signatureProvider) | |
| 547 | 555 | if isGenerated { | |
| 548 | 556 | sig, valid = p.EventSignature(f.opcode) | |
| 549 | 557 | } | |
| @@ -582,6 +590,8 @@ func (d *Display) prepareFrame(f receivedFrame) (Object, *Event, uint8, error) { | |||
| 582 | 590 | eventPool.Put(ev) | |
| 583 | 591 | return nil, nil, 0, &ProtocolError{Kind: "extra_fd", Object: f.object, Opcode: f.opcode, Err: ErrMalformedFrame} | |
| 584 | 592 | } | |
| 593 | + | // For a zombie the descriptors were still consumed above so the stream | |
| 594 | + | // stays aligned; dispatchFrame closes them without running any handler. | |
| 585 | 595 | return obj.(Object), ev, 0, nil | |
| 586 | 596 | } | |
| 587 | 597 | ||
| @@ -643,7 +653,11 @@ func (d *Display) handleDisplayEvent(opcode uint16, data []byte) error { | |||
| 643 | 653 | } | |
| 644 | 654 | id := binary.LittleEndian.Uint32(data[0:4]) | |
| 645 | 655 | d.idMu.Lock() | |
| 646 | - | _, live := d.objects.Load(id) | |
| 656 | + | obj, live := d.objects.Load(id) | |
| 657 | + | if _, isZombie := obj.(*zombie); isZombie { | |
| 658 | + | d.objects.CompareAndDelete(id, obj) | |
| 659 | + | live = false | |
| 660 | + | } | |
| 647 | 661 | if id < 2 || id >= 0xff000000 || id >= d.nextID || live || d.retiredIDs[id] { | |
| 648 | 662 | d.idMu.Unlock() | |
| 649 | 663 | return &ProtocolError{Kind: "invalid_delete_id", Object: id, Err: ErrMalformedFrame} | |
| @@ -500,6 +500,9 @@ func (d *Display) dispatchFrame(f receivedFrame) error { | |||||
| 500 | *ev = Event{} | 500 | *ev = Event{} | ||
| 501 | eventPool.Put(ev) | 501 | eventPool.Put(ev) | ||
| 502 | }() | 502 | }() | ||
| 503 | + | if _, isZombie := obj.(*zombie); isZombie { | |||
| 504 | + | return nil | |||
| 505 | + | } | |||
| 503 | if proxy, ok := obj.(Proxy); ok { | 506 | if proxy, ok := obj.(Proxy); ok { | ||
| 504 | proxy.Dispatch(ev) | 507 | proxy.Dispatch(ev) | ||
| 505 | } else { | 508 | } else { | ||
| @@ -542,8 +545,13 @@ func (d *Display) prepareFrame(f receivedFrame) (Object, *Event, uint8, error) { | |||||
| 542 | if !ok { | 545 | if !ok { | ||
| 543 | return nil, nil, 0, &ProtocolError{Kind: "unknown_object", Object: f.object, Opcode: f.opcode, Err: ErrUnknownObject} | 546 | return nil, nil, 0, &ProtocolError{Kind: "unknown_object", Object: f.object, Opcode: f.opcode, Err: ErrUnknownObject} | ||
| 544 | } | 547 | } | ||
| 548 | + | sigObj := obj | |||
| 549 | + | z, isZombie := obj.(*zombie) | |||
| 550 | + | if isZombie { | |||
| 551 | + | sigObj = z.object | |||
| 552 | + | } | |||
| 545 | sig, valid := "", false | 553 | sig, valid := "", false | ||
| 546 | - | p, isGenerated := obj.(signatureProvider) | 554 | + | p, isGenerated := sigObj.(signatureProvider) |
| 547 | if isGenerated { | 555 | if isGenerated { | ||
| 548 | sig, valid = p.EventSignature(f.opcode) | 556 | sig, valid = p.EventSignature(f.opcode) | ||
| 549 | } | 557 | } | ||
| @@ -582,6 +590,8 @@ func (d *Display) prepareFrame(f receivedFrame) (Object, *Event, uint8, error) { | |||||
| 582 | eventPool.Put(ev) | 590 | eventPool.Put(ev) | ||
| 583 | return nil, nil, 0, &ProtocolError{Kind: "extra_fd", Object: f.object, Opcode: f.opcode, Err: ErrMalformedFrame} | 591 | return nil, nil, 0, &ProtocolError{Kind: "extra_fd", Object: f.object, Opcode: f.opcode, Err: ErrMalformedFrame} | ||
| 584 | } | 592 | } | ||
| 593 | + | // For a zombie the descriptors were still consumed above so the stream | |||
| 594 | + | // stays aligned; dispatchFrame closes them without running any handler. | |||
| 585 | return obj.(Object), ev, 0, nil | 595 | return obj.(Object), ev, 0, nil | ||
| 586 | } | 596 | } | ||
| 587 | 597 | ||||
| @@ -643,7 +653,11 @@ func (d *Display) handleDisplayEvent(opcode uint16, data []byte) error { | |||||
| 643 | } | 653 | } | ||
| 644 | id := binary.LittleEndian.Uint32(data[0:4]) | 654 | id := binary.LittleEndian.Uint32(data[0:4]) | ||
| 645 | d.idMu.Lock() | 655 | d.idMu.Lock() | ||
| 646 | - | _, live := d.objects.Load(id) | 656 | + | obj, live := d.objects.Load(id) |
| 657 | + | if _, isZombie := obj.(*zombie); isZombie { | |||
| 658 | + | d.objects.CompareAndDelete(id, obj) | |||
| 659 | + | live = false | |||
| 660 | + | } | |||
| 647 | if id < 2 || id >= 0xff000000 || id >= d.nextID || live || d.retiredIDs[id] { | 661 | if id < 2 || id >= 0xff000000 || id >= d.nextID || live || d.retiredIDs[id] { | ||
| 648 | d.idMu.Unlock() | 662 | d.idMu.Unlock() | ||
| 649 | return &ProtocolError{Kind: "invalid_delete_id", Object: id, Err: ErrMalformedFrame} | 663 | return &ProtocolError{Kind: "invalid_delete_id", Object: id, Err: ErrMalformedFrame} | ||
M core.go +10 -1
| @@ -115,10 +115,19 @@ func (c *Context) claimDestroy(proxy Proxy) error { | |||
| 115 | 115 | if !c.proxies.CompareAndDelete(proxy.ID(), proxy) { | |
| 116 | 116 | return errors.New("proxy is not registered") | |
| 117 | 117 | } | |
| 118 | - | c.display.objects.CompareAndDelete(proxy.ID(), proxy) | |
| 118 | + | // The compositor may already have sent events for this object. Keep a | |
| 119 | + | // zombie until delete_id so those events are dropped, not fatal. | |
| 120 | + | c.display.objects.CompareAndSwap(proxy.ID(), proxy, &zombie{object: proxy}) | |
| 119 | 121 | return nil | |
| 120 | 122 | } | |
| 121 | 123 | ||
| 124 | + | // zombie stands in for an object the client destroyed until the compositor | |
| 125 | + | // acknowledges the destruction with wl_display.delete_id. Events that were in | |
| 126 | + | // flight for it are discarded; their descriptors are closed. | |
| 127 | + | type zombie struct{ object Object } | |
| 128 | + | ||
| 129 | + | func (z *zombie) ID() uint32 { return z.object.ID() } | |
| 130 | + | ||
| 122 | 131 | // Register registers a proxy object | |
| 123 | 132 | func (c *Context) Register(proxy Proxy) { | |
| 124 | 133 | if proxy != nil && proxy.ID() != 0 && !c.closed.Load() { | |
| @@ -115,10 +115,19 @@ func (c *Context) claimDestroy(proxy Proxy) error { | |||||
| 115 | if !c.proxies.CompareAndDelete(proxy.ID(), proxy) { | 115 | if !c.proxies.CompareAndDelete(proxy.ID(), proxy) { | ||
| 116 | return errors.New("proxy is not registered") | 116 | return errors.New("proxy is not registered") | ||
| 117 | } | 117 | } | ||
| 118 | - | c.display.objects.CompareAndDelete(proxy.ID(), proxy) | 118 | + | // The compositor may already have sent events for this object. Keep a |
| 119 | + | // zombie until delete_id so those events are dropped, not fatal. | |||
| 120 | + | c.display.objects.CompareAndSwap(proxy.ID(), proxy, &zombie{object: proxy}) | |||
| 119 | return nil | 121 | return nil | ||
| 120 | } | 122 | } | ||
| 121 | 123 | ||||
| 124 | + | // zombie stands in for an object the client destroyed until the compositor | |||
| 125 | + | // acknowledges the destruction with wl_display.delete_id. Events that were in | |||
| 126 | + | // flight for it are discarded; their descriptors are closed. | |||
| 127 | + | type zombie struct{ object Object } | |||
| 128 | + | ||||
| 129 | + | func (z *zombie) ID() uint32 { return z.object.ID() } | |||
| 130 | + | ||||
| 122 | // Register registers a proxy object | 131 | // Register registers a proxy object | ||
| 123 | func (c *Context) Register(proxy Proxy) { | 132 | func (c *Context) Register(proxy Proxy) { | ||
| 124 | if proxy != nil && proxy.ID() != 0 && !c.closed.Load() { | 133 | if proxy != nil && proxy.ID() != 0 && !c.closed.Load() { | ||
M hardening_test.go +70 -0
| @@ -2,7 +2,9 @@ package wlturbo | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "bytes" | |
| 5 | + | "encoding/binary" | |
| 5 | 6 | "errors" | |
| 7 | + | "io" | |
| 6 | 8 | "net" | |
| 7 | 9 | "os" | |
| 8 | 10 | "testing" | |
| @@ -55,6 +57,74 @@ func TestRegistryGlobalHandlersAccumulate(t *testing.T) { | |||
| 55 | 57 | } | |
| 56 | 58 | } | |
| 57 | 59 | ||
| 60 | + | type fdSigProxy struct { | |
| 61 | + | BaseProxy | |
| 62 | + | calls int | |
| 63 | + | } | |
| 64 | + | ||
| 65 | + | func (p *fdSigProxy) EventSignature(op uint16) (string, bool) { | |
| 66 | + | switch op { | |
| 67 | + | case 0: | |
| 68 | + | return "uint,", true | |
| 69 | + | case 1: | |
| 70 | + | return "fd,", true | |
| 71 | + | } | |
| 72 | + | return "", false | |
| 73 | + | } | |
| 74 | + | func (p *fdSigProxy) Dispatch(*Event) { p.calls++ } | |
| 75 | + | ||
| 76 | + | // Events already in flight for an object the client destroyed are dropped, | |
| 77 | + | // with their descriptors closed, until the compositor sends delete_id. | |
| 78 | + | func TestDestroyedObjectEventsAreDroppedUntilDeleteID(t *testing.T) { | |
| 79 | + | client, peer := unixSocketPair(t) | |
| 80 | + | d := newDisplay(client) | |
| 81 | + | p := &fdSigProxy{BaseProxy: BaseProxy{id: d.AllocateID(), context: d.context}} | |
| 82 | + | d.context.Register(p) | |
| 83 | + | if err := d.context.SendDestructor(p, 0); err != nil { | |
| 84 | + | t.Fatal(err) | |
| 85 | + | } | |
| 86 | + | if err := d.context.SendRequest(p, 1); err == nil { | |
| 87 | + | t.Fatal("request on destroyed proxy succeeded") | |
| 88 | + | } | |
| 89 | + | ||
| 90 | + | r, w, err := os.Pipe() | |
| 91 | + | if err != nil { | |
| 92 | + | t.Fatal(err) | |
| 93 | + | } | |
| 94 | + | defer r.Close() | |
| 95 | + | sendMessageWithFDs(t, peer, message(p.ID(), 0, []byte{1, 0, 0, 0}), nil) | |
| 96 | + | sendMessageWithFDs(t, peer, message(p.ID(), 1, nil), []int{int(w.Fd())}) | |
| 97 | + | w.Close() | |
| 98 | + | for i := 0; i < 2; i++ { | |
| 99 | + | if err := d.Dispatch(); err != nil { | |
| 100 | + | t.Fatalf("in-flight event %d: %v", i, err) | |
| 101 | + | } | |
| 102 | + | } | |
| 103 | + | if p.calls != 0 { | |
| 104 | + | t.Fatalf("destroyed proxy received %d events", p.calls) | |
| 105 | + | } | |
| 106 | + | if err := r.SetReadDeadline(time.Now().Add(2 * time.Second)); err != nil { | |
| 107 | + | t.Fatal(err) | |
| 108 | + | } | |
| 109 | + | if _, err := r.Read(make([]byte, 1)); !errors.Is(err, io.EOF) { | |
| 110 | + | t.Fatalf("descriptor for destroyed object not closed: %v", err) | |
| 111 | + | } | |
| 112 | + | ||
| 113 | + | body := make([]byte, 4) | |
| 114 | + | binary.LittleEndian.PutUint32(body, p.ID()) | |
| 115 | + | sendMessageWithFDs(t, peer, message(displayObjectID, 1, body), nil) | |
| 116 | + | if err := d.Dispatch(); err != nil { | |
| 117 | + | t.Fatalf("delete_id for zombie: %v", err) | |
| 118 | + | } | |
| 119 | + | sendMessageWithFDs(t, peer, message(p.ID(), 0, []byte{1, 0, 0, 0}), nil) | |
| 120 | + | if err := d.Dispatch(); !errors.Is(err, ErrUnknownObject) { | |
| 121 | + | t.Fatalf("event after delete_id = %v, want ErrUnknownObject", err) | |
| 122 | + | } | |
| 123 | + | if got := d.AllocateID(); got != p.ID() { | |
| 124 | + | t.Fatalf("ID %d not reused after delete_id (got %d)", p.ID(), got) | |
| 125 | + | } | |
| 126 | + | } | |
| 127 | + | ||
| 58 | 128 | // A plain Go int is ambiguous (value or descriptor) and must be rejected | |
| 59 | 129 | // instead of being written as zero. | |
| 60 | 130 | func TestMarshalRejectsPlainInt(t *testing.T) { | |
| @@ -2,7 +2,9 @@ package wlturbo | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "bytes" | 4 | "bytes" | ||
| 5 | + | "encoding/binary" | |||
| 5 | "errors" | 6 | "errors" | ||
| 7 | + | "io" | |||
| 6 | "net" | 8 | "net" | ||
| 7 | "os" | 9 | "os" | ||
| 8 | "testing" | 10 | "testing" | ||
| @@ -55,6 +57,74 @@ func TestRegistryGlobalHandlersAccumulate(t *testing.T) { | |||||
| 55 | } | 57 | } | ||
| 56 | } | 58 | } | ||
| 57 | 59 | ||||
| 60 | + | type fdSigProxy struct { | |||
| 61 | + | BaseProxy | |||
| 62 | + | calls int | |||
| 63 | + | } | |||
| 64 | + | ||||
| 65 | + | func (p *fdSigProxy) EventSignature(op uint16) (string, bool) { | |||
| 66 | + | switch op { | |||
| 67 | + | case 0: | |||
| 68 | + | return "uint,", true | |||
| 69 | + | case 1: | |||
| 70 | + | return "fd,", true | |||
| 71 | + | } | |||
| 72 | + | return "", false | |||
| 73 | + | } | |||
| 74 | + | func (p *fdSigProxy) Dispatch(*Event) { p.calls++ } | |||
| 75 | + | ||||
| 76 | + | // Events already in flight for an object the client destroyed are dropped, | |||
| 77 | + | // with their descriptors closed, until the compositor sends delete_id. | |||
| 78 | + | func TestDestroyedObjectEventsAreDroppedUntilDeleteID(t *testing.T) { | |||
| 79 | + | client, peer := unixSocketPair(t) | |||
| 80 | + | d := newDisplay(client) | |||
| 81 | + | p := &fdSigProxy{BaseProxy: BaseProxy{id: d.AllocateID(), context: d.context}} | |||
| 82 | + | d.context.Register(p) | |||
| 83 | + | if err := d.context.SendDestructor(p, 0); err != nil { | |||
| 84 | + | t.Fatal(err) | |||
| 85 | + | } | |||
| 86 | + | if err := d.context.SendRequest(p, 1); err == nil { | |||
| 87 | + | t.Fatal("request on destroyed proxy succeeded") | |||
| 88 | + | } | |||
| 89 | + | ||||
| 90 | + | r, w, err := os.Pipe() | |||
| 91 | + | if err != nil { | |||
| 92 | + | t.Fatal(err) | |||
| 93 | + | } | |||
| 94 | + | defer r.Close() | |||
| 95 | + | sendMessageWithFDs(t, peer, message(p.ID(), 0, []byte{1, 0, 0, 0}), nil) | |||
| 96 | + | sendMessageWithFDs(t, peer, message(p.ID(), 1, nil), []int{int(w.Fd())}) | |||
| 97 | + | w.Close() | |||
| 98 | + | for i := 0; i < 2; i++ { | |||
| 99 | + | if err := d.Dispatch(); err != nil { | |||
| 100 | + | t.Fatalf("in-flight event %d: %v", i, err) | |||
| 101 | + | } | |||
| 102 | + | } | |||
| 103 | + | if p.calls != 0 { | |||
| 104 | + | t.Fatalf("destroyed proxy received %d events", p.calls) | |||
| 105 | + | } | |||
| 106 | + | if err := r.SetReadDeadline(time.Now().Add(2 * time.Second)); err != nil { | |||
| 107 | + | t.Fatal(err) | |||
| 108 | + | } | |||
| 109 | + | if _, err := r.Read(make([]byte, 1)); !errors.Is(err, io.EOF) { | |||
| 110 | + | t.Fatalf("descriptor for destroyed object not closed: %v", err) | |||
| 111 | + | } | |||
| 112 | + | ||||
| 113 | + | body := make([]byte, 4) | |||
| 114 | + | binary.LittleEndian.PutUint32(body, p.ID()) | |||
| 115 | + | sendMessageWithFDs(t, peer, message(displayObjectID, 1, body), nil) | |||
| 116 | + | if err := d.Dispatch(); err != nil { | |||
| 117 | + | t.Fatalf("delete_id for zombie: %v", err) | |||
| 118 | + | } | |||
| 119 | + | sendMessageWithFDs(t, peer, message(p.ID(), 0, []byte{1, 0, 0, 0}), nil) | |||
| 120 | + | if err := d.Dispatch(); !errors.Is(err, ErrUnknownObject) { | |||
| 121 | + | t.Fatalf("event after delete_id = %v, want ErrUnknownObject", err) | |||
| 122 | + | } | |||
| 123 | + | if got := d.AllocateID(); got != p.ID() { | |||
| 124 | + | t.Fatalf("ID %d not reused after delete_id (got %d)", p.ID(), got) | |||
| 125 | + | } | |||
| 126 | + | } | |||
| 127 | + | ||||
| 58 | // A plain Go int is ambiguous (value or descriptor) and must be rejected | 128 | // A plain Go int is ambiguous (value or descriptor) and must be rejected | ||
| 59 | // instead of being written as zero. | 129 | // instead of being written as zero. | ||
| 60 | func TestMarshalRejectsPlainInt(t *testing.T) { | 130 | func TestMarshalRejectsPlainInt(t *testing.T) { | ||