Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(examplebroker): Use locked access to isAuthenticatedCalls on end session #568

Merged
merged 1 commit into from
Oct 3, 2024

Conversation

3v1n0
Copy link
Collaborator

@3v1n0 3v1n0 commented Oct 2, 2024

We had a data race, since we were accessing to the isAuthenticatedCalls map without locking, fix this avoiding lock/unlock dance.

WARNING: DATA RACE
Read at 0x00c0001a03f0 by goroutine 311:
	runtime.mapaccess2_faststr()
		/opt/hostedtoolcache/go/1.23.0/x64/src/runtime/map_faststr.go:117 +0x0
	github.com/ubuntu/authd/examplebroker.(*Broker).EndSession()
		/home/runner/work/authd/authd/examplebroker/broker.go:733 +0xf9
	github.com/ubuntu/authd/examplebroker.(*Bus).EndSession()
		/home/runner/work/authd/authd/examplebroker/dbus.go:115 +0x5b
	runtime.call32()
		/opt/hostedtoolcache/go/1.23.0/x64/src/runtime/asm_amd64.s:776 +0x42
	reflect.Value.Call()
		/opt/hostedtoolcache/go/1.23.0/x64/src/reflect/value.go:365 +0xb5
	github.com/godbus/dbus/v5.exportedMethod.Call()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/default_handler.go:128 +0x239
	github.com/godbus/dbus/v5.(*exportedMethod).Call()
		<autogenerated>:1 +0x84
	github.com/godbus/dbus/v5.(*Conn).handleCall()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/export.go:193 +0x6f2
	github.com/godbus/dbus/v5.(*Conn).inWorker.gowrap1()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/conn.go:435 +0x44

Previous write at 0x00c0001a03f0 by goroutine 308:
	runtime.mapassign_faststr()
		/opt/hostedtoolcache/go/1.23.0/x64/src/runtime/map_faststr.go:223 +0x0
	github.com/ubuntu/authd/examplebroker.(*Broker).IsAuthenticated()
		/home/runner/work/authd/authd/examplebroker/broker.go:537 +0x276
	github.com/ubuntu/authd/examplebroker.(*Bus).IsAuthenticated()
		/home/runner/work/authd/authd/examplebroker/dbus.go:106 +0x91
	runtime.call64()
		/opt/hostedtoolcache/go/1.23.0/x64/src/runtime/asm_amd64.s:777 +0x42
	reflect.Value.Call()
		/opt/hostedtoolcache/go/1.23.0/x64/src/reflect/value.go:365 +0xb5
	github.com/godbus/dbus/v5.exportedMethod.Call()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/default_handler.go:128 +0x239
	github.com/godbus/dbus/v5.(*exportedMethod).Call()
		<autogenerated>:1 +0x84
	github.com/godbus/dbus/v5.(*Conn).handleCall()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/export.go:193 +0x6f2
	github.com/godbus/dbus/v5.(*Conn).inWorker.gowrap1()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/conn.go:435 +0x44

Goroutine 311 (running) created at:
	github.com/godbus/dbus/v5.(*Conn).inWorker()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/conn.go:435 +0x46a
	github.com/godbus/dbus/v5.(*Conn).Auth.gowrap1()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/auth.go:118 +0x33

Goroutine 308 (running) created at:
	github.com/godbus/dbus/v5.(*Conn).inWorker()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/conn.go:435 +0x46a
	github.com/godbus/dbus/v5.(*Conn).Auth.gowrap1()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/auth.go:118 +0x33
	github.com/godbus/dbus/v5.(*Conn).inWorker.gowrap1()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/conn.go:435 +0x44

Previous write at 0x00c0001a03f0 by goroutine 308:
	runtime.mapassign_faststr()
		/opt/hostedtoolcache/go/1.23.0/x64/src/runtime/map_faststr.go:223 +0x0
	github.com/ubuntu/authd/examplebroker.(*Broker).IsAuthenticated()
		/home/runner/work/authd/authd/examplebroker/broker.go:537 +0x276
	github.com/ubuntu/authd/examplebroker.(*Bus).IsAuthenticated()
		/home/runner/work/authd/authd/examplebroker/dbus.go:106 +0x91
	runtime.call64()
		/opt/hostedtoolcache/go/1.23.0/x64/src/runtime/asm_amd64.s:777 +0x42
	reflect.Value.Call()
		/opt/hostedtoolcache/go/1.23.0/x64/src/reflect/value.go:365 +0xb5
	github.com/godbus/dbus/v5.exportedMethod.Call()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/default_handler.go:128 +0x239
	github.com/godbus/dbus/v5.(*exportedMethod).Call()
		<autogenerated>:1 +0x84
	github.com/godbus/dbus/v5.(*Conn).handleCall()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/export.go:193 +0x6f2
	github.com/godbus/dbus/v5.(*Conn).inWorker.gowrap1()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/conn.go:435 +0x44

Goroutine 311 (running) created at:
	github.com/godbus/dbus/v5.(*Conn).inWorker()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/conn.go:435 +0x46a
	github.com/godbus/dbus/v5.(*Conn).Auth.gowrap1()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/auth.go:118 +0x33

Goroutine 308 (running) created at:
	github.com/godbus/dbus/v5.(*Conn).inWorker()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/conn.go:435 +0x46a
	github.com/godbus/dbus/v5.(*Conn).Auth.gowrap1()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/auth.go:118 +0x33

See: https://github.com/ubuntu/authd/actions/runs/11153104769/job/30999989707

@3v1n0 3v1n0 requested a review from a team as a code owner October 2, 2024 23:57
We had a data race, since we were accessing to the isAuthenticatedCalls
map without locking, fix this avoiding lock/unlock dance.

WARNING: DATA RACE
Read at 0x00c0001a03f0 by goroutine 311:
	runtime.mapaccess2_faststr()
		/opt/hostedtoolcache/go/1.23.0/x64/src/runtime/map_faststr.go:117 +0x0
	github.com/ubuntu/authd/examplebroker.(*Broker).EndSession()
		/home/runner/work/authd/authd/examplebroker/broker.go:733 +0xf9
	github.com/ubuntu/authd/examplebroker.(*Bus).EndSession()
		/home/runner/work/authd/authd/examplebroker/dbus.go:115 +0x5b
	runtime.call32()
		/opt/hostedtoolcache/go/1.23.0/x64/src/runtime/asm_amd64.s:776 +0x42
	reflect.Value.Call()
		/opt/hostedtoolcache/go/1.23.0/x64/src/reflect/value.go:365 +0xb5
	github.com/godbus/dbus/v5.exportedMethod.Call()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/default_handler.go:128 +0x239
	github.com/godbus/dbus/v5.(*exportedMethod).Call()
		<autogenerated>:1 +0x84
	github.com/godbus/dbus/v5.(*Conn).handleCall()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/export.go:193 +0x6f2
	github.com/godbus/dbus/v5.(*Conn).inWorker.gowrap1()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/conn.go:435 +0x44

Previous write at 0x00c0001a03f0 by goroutine 308:
	runtime.mapassign_faststr()
		/opt/hostedtoolcache/go/1.23.0/x64/src/runtime/map_faststr.go:223 +0x0
	github.com/ubuntu/authd/examplebroker.(*Broker).IsAuthenticated()
		/home/runner/work/authd/authd/examplebroker/broker.go:537 +0x276
	github.com/ubuntu/authd/examplebroker.(*Bus).IsAuthenticated()
		/home/runner/work/authd/authd/examplebroker/dbus.go:106 +0x91
	runtime.call64()
		/opt/hostedtoolcache/go/1.23.0/x64/src/runtime/asm_amd64.s:777 +0x42
	reflect.Value.Call()
		/opt/hostedtoolcache/go/1.23.0/x64/src/reflect/value.go:365 +0xb5
	github.com/godbus/dbus/v5.exportedMethod.Call()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/default_handler.go:128 +0x239
	github.com/godbus/dbus/v5.(*exportedMethod).Call()
		<autogenerated>:1 +0x84
	github.com/godbus/dbus/v5.(*Conn).handleCall()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/export.go:193 +0x6f2
	github.com/godbus/dbus/v5.(*Conn).inWorker.gowrap1()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/conn.go:435 +0x44

Goroutine 311 (running) created at:
	github.com/godbus/dbus/v5.(*Conn).inWorker()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/conn.go:435 +0x46a
	github.com/godbus/dbus/v5.(*Conn).Auth.gowrap1()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/auth.go:118 +0x33

Goroutine 308 (running) created at:
	github.com/godbus/dbus/v5.(*Conn).inWorker()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/conn.go:435 +0x46a
	github.com/godbus/dbus/v5.(*Conn).Auth.gowrap1()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/auth.go:118 +0x33
	github.com/godbus/dbus/v5.(*Conn).inWorker.gowrap1()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/conn.go:435 +0x44

Previous write at 0x00c0001a03f0 by goroutine 308:
	runtime.mapassign_faststr()
		/opt/hostedtoolcache/go/1.23.0/x64/src/runtime/map_faststr.go:223 +0x0
	github.com/ubuntu/authd/examplebroker.(*Broker).IsAuthenticated()
		/home/runner/work/authd/authd/examplebroker/broker.go:537 +0x276
	github.com/ubuntu/authd/examplebroker.(*Bus).IsAuthenticated()
		/home/runner/work/authd/authd/examplebroker/dbus.go:106 +0x91
	runtime.call64()
		/opt/hostedtoolcache/go/1.23.0/x64/src/runtime/asm_amd64.s:777 +0x42
	reflect.Value.Call()
		/opt/hostedtoolcache/go/1.23.0/x64/src/reflect/value.go:365 +0xb5
	github.com/godbus/dbus/v5.exportedMethod.Call()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/default_handler.go:128 +0x239
	github.com/godbus/dbus/v5.(*exportedMethod).Call()
		<autogenerated>:1 +0x84
	github.com/godbus/dbus/v5.(*Conn).handleCall()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/export.go:193 +0x6f2
	github.com/godbus/dbus/v5.(*Conn).inWorker.gowrap1()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/conn.go:435 +0x44

Goroutine 311 (running) created at:
	github.com/godbus/dbus/v5.(*Conn).inWorker()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/conn.go:435 +0x46a
	github.com/godbus/dbus/v5.(*Conn).Auth.gowrap1()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/auth.go:118 +0x33

Goroutine 308 (running) created at:
	github.com/godbus/dbus/v5.(*Conn).inWorker()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/conn.go:435 +0x46a
	github.com/godbus/dbus/v5.(*Conn).Auth.gowrap1()
		/home/runner/go/pkg/mod/github.com/godbus/dbus/[email protected]/auth.go:118 +0x33
@3v1n0 3v1n0 force-pushed the example-broker-cancel-race branch from f600537 to 01e2dc5 Compare October 3, 2024 00:01
@3v1n0 3v1n0 merged commit 0354d40 into ubuntu:main Oct 3, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants