Skip to content

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

Description

@OvOhao

Possible memory leak of every string allocated through Calloc.String on the v2 linux 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/linux/calloc.go:13

// 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.

The one user in this package is OpenFileDialog,
v2/internal/frontend/desktop/linux/window.go:388:

	if len(dialogOptions.Filters) > 0 {
		// Create filter array
		mem := NewCalloc()
		arraySize := len(dialogOptions.Filters) + 1
		data.filters = C.AllocFileFilterArray((C.size_t)(arraySize))
		filters := unsafe.Slice((**C.struct__GtkFileFilter)(unsafe.Pointer(data.filters)), arraySize)
		for index, filter := range dialogOptions.Filters {
			thisFilter := C.gtk_file_filter_new()
			C.g_object_ref(C.gpointer(thisFilter))
			if filter.DisplayName != "" {
				cName := mem.String(filter.DisplayName)
				C.gtk_file_filter_set_name(thisFilter, cName)
			}
			if filter.Pattern != "" {
				for _, thisPattern := range strings.Split(filter.Pattern, ";") {
					cThisPattern := mem.String(thisPattern)
					C.gtk_file_filter_add_pattern(thisFilter, cThisPattern)
				}
			}
			// Add filter to array
			filters[index] = thisFilter
		}
		mem.Free()
		filters[arraySize-1] = nil
	}

gtk_file_filter_set_name and gtk_file_filter_add_pattern both copy their argument, so
the caller owns the strings and mem.Free() is the only thing that could release them.
It releases nothing. One dialog with N filters leaks one string per display name plus one
per pattern, and the dialog can be opened repeatedly.

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() { ... }

The call site needs no change.

The identical value-receiver Calloc also exists in
v2/internal/frontend/desktop/darwin/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