Skip to content

Lock leak in UpdateMetadata and ExtendMetadata (on parseMetadata error) #3010

@palkeo

Description

@palkeo

Is there an existing issue for this?

  • There is no existing issue for this bug

Is this happening on an up to date version of Incus?

  • This is happening on a supported version of Incus

Incus system details

latest master branch

Instance details

No response

Instance log

No response

Current behavior

Small issue found in the code, I'm not sure how feasible it is to trigger it, but it looks obviously wrong to me, so I'm reporting it.

UpdateMetadata and ExtendMetadata in internal/server/operations/operations.go can leak op.lock if parseMetadata returns an error. The lock is acquired before the call but the error return path doesn't release it.
All the other error paths do release it.

UpdateMetadata (lines 585–599):

func (op *Operation) UpdateMetadata(opMetadata any) error {
	op.lock.Lock()                              // lock acquired
	// ... status/readonly checks with Unlock ...

	newMetadata, err := parseMetadata(opMetadata)
	if err != nil {
		return err                              // ← returns without Unlock
	}

ExtendMetadata (lines 619–635):

func (op *Operation) ExtendMetadata(metadata any) error {
	op.lock.Lock()                              // lock acquired
	// ... status/readonly checks with Unlock ...

	extraMetadata, err := parseMetadata(metadata)
	if err != nil {
		return err                              // ← returns without Unlock
	}

How it was introduced:

Commit ad73f87a6 ("lxd/operations: Fix possible races on accessing status property", May 2022) moved op.lock.Lock() above the status and readonly checks so that reading op.status and op.readonly would be protected by the lock. This was correct, but as a side-effect the parseMetadata() call — which previously ran before the lock was acquired — ended up inside the locked section. The commit added op.lock.Unlock() for the status/readonly early-return paths but missed the parseMetadata error return path.

PS: this was found by an LLM while doing an unrelated investigation. Reporting because it sounds obviously buggy to me, to the best of my knowledge.

Expected behavior

No response

Steps to reproduce

.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions