Skip to content

Possible memory leak of every string allocated through Calloc.String on the v2 darwin frontend #6058

Description

@OvOhao

Possible memory leak of every string allocated through Calloc.String on the v2 darwin frontend

Calloc.String has a value receiver, so the append writes to a copy of the receiver
and the caller's pool stays empty. Calloc.Free then iterates an empty slice and frees
nothing. Every C.CString handed out by this allocator leaks.

v2/internal/frontend/desktop/darwin/calloc.go:12

// Calloc handles alloc/dealloc of C data
type Calloc struct {
	pool []unsafe.Pointer
}

// NewCalloc creates a new allocator
func NewCalloc() Calloc {
	return Calloc{}
}

// String creates a new C string and retains a reference to it
func (c Calloc) String(in string) *C.char {
	result := C.CString(in)
	c.pool = append(c.pool, unsafe.Pointer(result))
	return result
}

// Free frees all allocated C memory
func (c Calloc) Free() {
	for _, str := range c.pool {
		C.free(str)
	}
	c.pool = []unsafe.Pointer{}
}

NewCalloc returns a Calloc value whose pool is nil. String takes the receiver by
value, so c.pool = append(...) updates a copy that is discarded when String returns.
The caller's pool is still nil at Free, and range over a nil slice runs zero
iterations.

Every call site is affected. v2/internal/frontend/desktop/darwin/dialog.go:47:

	c := NewCalloc()
	defer c.Free()
	title := c.String(options.Title)
	defaultFilename := c.String(options.DefaultFilename)
	defaultDirectory := c.String(options.DefaultDirectory)

and v2/internal/frontend/desktop/darwin/menu.go:29:

func NewNSMenu(context unsafe.Pointer, name string) *NSMenu {
	c := NewCalloc()
	defer c.Free()
	title := c.String(name)
	nsmenu := C.NewMenu(title)

defer c.Free() also evaluates its receiver at the defer statement, so it captures the
copy taken before any String call. The other NewCalloc users are dialog.go:111,
dialog.go:146, menu.go:58, single_instance.go:31 and window.go:52; every string they
allocate leaks for the process lifetime.

The same defect was already fixed on the v3 side. v3/pkg/application/linux_cgo.go:28 now
reads:

// NewCalloc creates a new allocator. Returns a pointer so the allocation
// pool is shared across calls without copying.
func NewCalloc() *Calloc {
	return &Calloc{}
}

// String creates a new C string and retains a reference to it.
func (c *Calloc) String(in string) *C.char {
	result := C.CString(in)
	c.pool = append(c.pool, unsafe.Pointer(result))
	return result
}

The v2 copy did not get the same change.

Fix, matching what v3 already does:

func NewCalloc() *Calloc {
	return &Calloc{}
}

func (c *Calloc) String(in string) *C.char { ... }

func (c *Calloc) Free() { ... }

Call sites need no change: c := NewCalloc(); defer c.Free() keeps working once
NewCalloc returns a pointer.

The identical value-receiver Calloc also exists in
v2/internal/frontend/desktop/linux/calloc.go; I have opened a separate issue for it.

If you could credit me as a reporter for my contributions to security advisory I will be thankful.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions