fix(transport): reject fd sends before claiming and close the socket on write failure
- commit
- 1cc5bed41b0d5c0503c997c534e4ae9b458f9ef5
- author
- brice · · 2026-09-28 07:48 UTC
- parent
- b2fe39a
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +20 -11
M protocol/core/core_test.go +6 -0
| @@ -441,6 +441,7 @@ func TestConcurrentDestroySendsOnce(t *testing.T) { | |||
| 441 | 441 | defer d.Close() | |
| 442 | 442 | receiveRequest(t, p) | |
| 443 | 443 | const rounds, destroyers, committers = 100, 4, 4 | |
| 444 | + | totalCommits := 0 | |
| 444 | 445 | for round := 0; round < rounds; round++ { | |
| 445 | 446 | surface := core.NewSurface(d.Context()) | |
| 446 | 447 | surface.SetID(d.AllocateID()) | |
| @@ -469,6 +470,7 @@ func TestConcurrentDestroySendsOnce(t *testing.T) { | |||
| 469 | 470 | if okDestroy != 1 { | |
| 470 | 471 | t.Fatalf("round %d: %d destroys succeeded, want 1", round, okDestroy) | |
| 471 | 472 | } | |
| 473 | + | totalCommits += okCommit | |
| 472 | 474 | // Decode every frame: exactly okCommit commits (opcode 6), then one | |
| 473 | 475 | // destroy (opcode 0) for this surface, and nothing after it. | |
| 474 | 476 | frames := readFrames(t, p, okCommit+1) | |
| @@ -487,6 +489,10 @@ func TestConcurrentDestroySendsOnce(t *testing.T) { | |||
| 487 | 489 | } | |
| 488 | 490 | p.SetReadDeadline(time.Time{}) | |
| 489 | 491 | } | |
| 492 | + | // Ordering is only exercised when some commits won the race. | |
| 493 | + | if totalCommits == 0 { | |
| 494 | + | t.Fatal("no commit succeeded in any round; ordering was not exercised") | |
| 495 | + | } | |
| 490 | 496 | } | |
| 491 | 497 | ||
| 492 | 498 | type wireFrame struct { | |
| @@ -441,6 +441,7 @@ func TestConcurrentDestroySendsOnce(t *testing.T) { | |||||
| 441 | defer d.Close() | 441 | defer d.Close() | ||
| 442 | receiveRequest(t, p) | 442 | receiveRequest(t, p) | ||
| 443 | const rounds, destroyers, committers = 100, 4, 4 | 443 | const rounds, destroyers, committers = 100, 4, 4 | ||
| 444 | + | totalCommits := 0 | |||
| 444 | for round := 0; round < rounds; round++ { | 445 | for round := 0; round < rounds; round++ { | ||
| 445 | surface := core.NewSurface(d.Context()) | 446 | surface := core.NewSurface(d.Context()) | ||
| 446 | surface.SetID(d.AllocateID()) | 447 | surface.SetID(d.AllocateID()) | ||
| @@ -469,6 +470,7 @@ func TestConcurrentDestroySendsOnce(t *testing.T) { | |||||
| 469 | if okDestroy != 1 { | 470 | if okDestroy != 1 { | ||
| 470 | t.Fatalf("round %d: %d destroys succeeded, want 1", round, okDestroy) | 471 | t.Fatalf("round %d: %d destroys succeeded, want 1", round, okDestroy) | ||
| 471 | } | 472 | } | ||
| 473 | + | totalCommits += okCommit | |||
| 472 | // Decode every frame: exactly okCommit commits (opcode 6), then one | 474 | // Decode every frame: exactly okCommit commits (opcode 6), then one | ||
| 473 | // destroy (opcode 0) for this surface, and nothing after it. | 475 | // destroy (opcode 0) for this surface, and nothing after it. | ||
| 474 | frames := readFrames(t, p, okCommit+1) | 476 | frames := readFrames(t, p, okCommit+1) | ||
| @@ -487,6 +489,10 @@ func TestConcurrentDestroySendsOnce(t *testing.T) { | |||||
| 487 | } | 489 | } | ||
| 488 | p.SetReadDeadline(time.Time{}) | 490 | p.SetReadDeadline(time.Time{}) | ||
| 489 | } | 491 | } | ||
| 492 | + | // Ordering is only exercised when some commits won the race. | |||
| 493 | + | if totalCommits == 0 { | |||
| 494 | + | t.Fatal("no commit succeeded in any round; ordering was not exercised") | |||
| 495 | + | } | |||
| 490 | } | 496 | } | ||
| 491 | 497 | ||||
| 492 | type wireFrame struct { | 498 | type wireFrame struct { | ||
M scm_linux.go +14 -11
| @@ -143,25 +143,28 @@ func (d *Display) sendmsgWithFDs(buf []byte, fds []int, guard func() error) erro | |||
| 143 | 143 | if d.closed.Load() { | |
| 144 | 144 | return net.ErrClosed | |
| 145 | 145 | } | |
| 146 | + | if len(fds) > 0 && d.unix == nil { | |
| 147 | + | return errors.New("wlturbo: cannot send file descriptors over a non-Unix connection") | |
| 148 | + | } | |
| 146 | 149 | if guard != nil { | |
| 147 | 150 | if err := guard(); err != nil { | |
| 148 | 151 | return err | |
| 149 | 152 | } | |
| 150 | 153 | } | |
| 154 | + | var n int | |
| 155 | + | var err error | |
| 151 | 156 | if len(fds) == 0 { | |
| 152 | - | n, err := d.conn.Write(buf) | |
| 153 | - | if err == nil && n != len(buf) { | |
| 154 | - | return io.ErrShortWrite | |
| 155 | - | } | |
| 156 | - | return err | |
| 157 | + | n, err = d.conn.Write(buf) | |
| 158 | + | } else { | |
| 159 | + | n, _, err = d.unix.WriteMsgUnix(buf, unix.UnixRights(fds...), nil) | |
| 157 | 160 | } | |
| 158 | - | if d.unix == nil { | |
| 159 | - | return errors.New("wlturbo: cannot send file descriptors over a non-Unix connection") | |
| 160 | - | } | |
| 161 | - | ||
| 162 | - | n, _, err := d.unix.WriteMsgUnix(buf, unix.UnixRights(fds...), nil) | |
| 163 | 161 | if err == nil && n != len(buf) { | |
| 164 | - | return io.ErrShortWrite | |
| 162 | + | err = io.ErrShortWrite | |
| 163 | + | } | |
| 164 | + | if err != nil { | |
| 165 | + | // A failed or partial write desynchronizes the stream: shut the socket | |
| 166 | + | // down so every later send and receive fails instead of corrupting it. | |
| 167 | + | _ = d.conn.Close() | |
| 165 | 168 | } | |
| 166 | 169 | return err | |
| 167 | 170 | } | |
| @@ -143,25 +143,28 @@ func (d *Display) sendmsgWithFDs(buf []byte, fds []int, guard func() error) erro | |||||
| 143 | if d.closed.Load() { | 143 | if d.closed.Load() { | ||
| 144 | return net.ErrClosed | 144 | return net.ErrClosed | ||
| 145 | } | 145 | } | ||
| 146 | + | if len(fds) > 0 && d.unix == nil { | |||
| 147 | + | return errors.New("wlturbo: cannot send file descriptors over a non-Unix connection") | |||
| 148 | + | } | |||
| 146 | if guard != nil { | 149 | if guard != nil { | ||
| 147 | if err := guard(); err != nil { | 150 | if err := guard(); err != nil { | ||
| 148 | return err | 151 | return err | ||
| 149 | } | 152 | } | ||
| 150 | } | 153 | } | ||
| 154 | + | var n int | |||
| 155 | + | var err error | |||
| 151 | if len(fds) == 0 { | 156 | if len(fds) == 0 { | ||
| 152 | - | n, err := d.conn.Write(buf) | 157 | + | n, err = d.conn.Write(buf) |
| 153 | - | if err == nil && n != len(buf) { | 158 | + | } else { |
| 154 | - | return io.ErrShortWrite | 159 | + | n, _, err = d.unix.WriteMsgUnix(buf, unix.UnixRights(fds...), nil) |
| 155 | - | } | |||
| 156 | - | return err | |||
| 157 | } | 160 | } | ||
| 158 | - | if d.unix == nil { | |||
| 159 | - | return errors.New("wlturbo: cannot send file descriptors over a non-Unix connection") | |||
| 160 | - | } | |||
| 161 | - | ||||
| 162 | - | n, _, err := d.unix.WriteMsgUnix(buf, unix.UnixRights(fds...), nil) | |||
| 163 | if err == nil && n != len(buf) { | 161 | if err == nil && n != len(buf) { | ||
| 164 | - | return io.ErrShortWrite | 162 | + | err = io.ErrShortWrite |
| 163 | + | } | |||
| 164 | + | if err != nil { | |||
| 165 | + | // A failed or partial write desynchronizes the stream: shut the socket | |||
| 166 | + | // down so every later send and receive fails instead of corrupting it. | |||
| 167 | + | _ = d.conn.Close() | |||
| 165 | } | 168 | } | ||
| 166 | return err | 169 | return err | ||
| 167 | } | 170 | } | ||