From 24f832e0325b3b65756ebbe7ddf4c2067a14bd90 Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Tue, 22 Sep 2026 19:50:21 +0200 Subject: [PATCH] Fix big-endian pixel swizzle, closed-capturer panic on FreeBSD, and the macOS agent socket dir symlink race --- client/vnc/server/agent_darwin.go | 18 ++++++++++-- client/vnc/server/capture_fb_freebsd.go | 9 ++++++ client/vnc/server/swizzle.go | 31 ++++++++++++++++++--- client/vnc/server/swizzle_test.go | 37 +++++++++++++++++++++++++ 4 files changed, 89 insertions(+), 6 deletions(-) create mode 100644 client/vnc/server/swizzle_test.go diff --git a/client/vnc/server/agent_darwin.go b/client/vnc/server/agent_darwin.go index 2c7abfd3e..b61eb63f8 100644 --- a/client/vnc/server/agent_darwin.go +++ b/client/vnc/server/agent_darwin.go @@ -180,10 +180,24 @@ func prepareAgentSocketDir(uid uint32) (string, error) { if err := os.Mkdir(subdir, 0o700); err != nil && !errors.Is(err, os.ErrExist) { return "", fmt.Errorf("mkdir %s: %w", subdir, err) } - if err := os.Chmod(subdir, 0o700); err != nil { + + // A sticky world-writable parent is accepted, and the sticky bit only stops + // another user replacing an entry — it does not stop them creating one. So + // between the purge above and this Mkdir they can put a symlink at + // vnc-, and Mkdir then returns EEXIST over it. Applying the mode and + // owner by path from here would follow that symlink and chown whatever it + // points at to them; going through an O_NOFOLLOW descriptor refuses the + // symlink outright and pins the rest to the directory actually opened. + f, err := os.OpenFile(subdir, os.O_RDONLY|syscall.O_NOFOLLOW|syscall.O_DIRECTORY, 0) + if err != nil { + return "", fmt.Errorf("open agent socket dir %s: %w", subdir, err) + } + defer f.Close() + + if err := f.Chmod(0o700); err != nil { return "", fmt.Errorf("chmod %s: %w", subdir, err) } - if err := os.Chown(subdir, int(uid), -1); err != nil { + if err := f.Chown(int(uid), -1); err != nil { return "", fmt.Errorf("chown %s -> uid %d: %w", subdir, uid, err) } return subdir, nil diff --git a/client/vnc/server/capture_fb_freebsd.go b/client/vnc/server/capture_fb_freebsd.go index 1896178b3..e0be37be2 100644 --- a/client/vnc/server/capture_fb_freebsd.go +++ b/client/vnc/server/capture_fb_freebsd.go @@ -3,6 +3,7 @@ package server import ( + "errors" "fmt" "image" "sync" @@ -140,6 +141,14 @@ func (c *FBCapturer) Capture() (*image.RGBA, error) { func (c *FBCapturer) CaptureInto(dst *image.RGBA) error { c.mu.Lock() defer c.mu.Unlock() + + // Close unmaps but leaves w/h in place, so a capture arriving afterwards + // would pass the size check below and index into a nil mapping. Matches the + // Linux capturer. + if c.mmap == nil { + return errors.New("framebuffer capturer is closed") + } + if dst.Rect.Dx() != c.w || dst.Rect.Dy() != c.h { return fmt.Errorf("dst size mismatch: dst=%dx%d fb=%dx%d", dst.Rect.Dx(), dst.Rect.Dy(), c.w, c.h) diff --git a/client/vnc/server/swizzle.go b/client/vnc/server/swizzle.go index 264a2f5a1..98c988a04 100644 --- a/client/vnc/server/swizzle.go +++ b/client/vnc/server/swizzle.go @@ -2,12 +2,18 @@ package server -import "unsafe" +import ( + "encoding/binary" + "unsafe" +) + +// nativeIsLittleEndian reports the byte order of the target. The word-at-a-time +// swizzle below only holds on a little-endian layout, and the client ships +// big-endian linux/mips and linux/mips64 builds. +var nativeIsLittleEndian = binary.NativeEndian.Uint16([]byte{1, 0}) == 1 // swizzleBGRAtoRGBA swaps B and R channels in a BGRA pixel buffer and copies -// into dst in-place (dst and src may alias). Operates on uint32 words: one -// read-modify-write per pixel, which is meaningfully faster than the naive -// three-byte-store per pixel for large buffers like framebuffers. +// into dst in-place (dst and src may alias). // // The alpha byte is forced to 0xff so callers that capture from X11 GetImage // (where the X server leaves the pad byte as zero) still get an opaque image. @@ -19,6 +25,14 @@ func swizzleBGRAtoRGBA(dst, src []byte) { if n == 0 { return } + if !nativeIsLittleEndian { + swizzleBGRAtoRGBABytes(dst[:n*4], src[:n*4]) + return + } + + // One read-modify-write per pixel, meaningfully faster than three byte + // stores over a whole framebuffer. The masks below describe a little-endian + // word, so this path is guarded above rather than used unconditionally. dp := unsafe.Slice((*uint32)(unsafe.Pointer(&dst[0])), n) sp := unsafe.Slice((*uint32)(unsafe.Pointer(&src[0])), n) for i := range n { @@ -28,3 +42,12 @@ func swizzleBGRAtoRGBA(dst, src []byte) { dp[i] = 0xFF000000 | (p & 0x0000FF00) | ((p & 0x00FF0000) >> 16) | ((p & 0x000000FF) << 16) } } + +// swizzleBGRAtoRGBABytes is the byte-order-independent form, used on big-endian +// targets. dst and src must be the same length and a multiple of 4. +func swizzleBGRAtoRGBABytes(dst, src []byte) { + for i := 0; i < len(src); i += 4 { + b, g, r := src[i], src[i+1], src[i+2] + dst[i], dst[i+1], dst[i+2], dst[i+3] = r, g, b, 0xFF + } +} diff --git a/client/vnc/server/swizzle_test.go b/client/vnc/server/swizzle_test.go new file mode 100644 index 000000000..8d70d3085 --- /dev/null +++ b/client/vnc/server/swizzle_test.go @@ -0,0 +1,37 @@ +//go:build !js && !ios && !android + +package server + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +// Both swizzle paths must agree: the word-at-a-time one runs on little-endian +// targets, the byte one on the big-endian mips builds the client also ships. +func TestSwizzleBGRAtoRGBA_PathsAgree(t *testing.T) { + src := []byte{ + 0x11, 0x22, 0x33, 0x00, // B G R pad + 0xAA, 0xBB, 0xCC, 0xFF, + } + want := []byte{ + 0x33, 0x22, 0x11, 0xFF, // R G B opaque + 0xCC, 0xBB, 0xAA, 0xFF, + } + + fast := make([]byte, len(src)) + swizzleBGRAtoRGBA(fast, src) + assert.Equal(t, want, fast, "swizzle must produce opaque RGBA") + + portable := make([]byte, len(src)) + swizzleBGRAtoRGBABytes(portable, src) + assert.Equal(t, want, portable, "the byte path must match the word path") +} + +// dst and src may alias; in-place must give the same answer. +func TestSwizzleBGRAtoRGBA_InPlace(t *testing.T) { + buf := []byte{0x11, 0x22, 0x33, 0x00} + swizzleBGRAtoRGBA(buf, buf) + assert.Equal(t, []byte{0x33, 0x22, 0x11, 0xFF}, buf) +}