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

hash/crc32: data race #41911

Closed
BenLubar opened this issue Oct 10, 2020 · 2 comments
Closed

hash/crc32: data race #41911

BenLubar opened this issue Oct 10, 2020 · 2 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.

Comments

@BenLubar
Copy link

The race detector reports a data race between these two lines:

https://github.com/golang/go/blob/go1.15.2/src/hash/crc32/crc32.go#L83
https://github.com/golang/go/blob/go1.15.2/src/hash/crc32/crc32.go#L226

Since the function at the first line will always have returned before the pointer stored in that variable is visible to anything, there are a few possible ways this data race could actually cause the wrong code to run, none of which are too exciting:

  • if d.tab is nil and castagnoliTable is nil, the wrong code path will run, but it'll crash on either code path with a nil pointer dereference.
  • if castagnoliTable is set non-atomically and just happens to match the exact bit pattern of d.tab, the wrong code path will run. this would require having a CRC table allocated on a 4 gibibyte boundary, assuming CPUs can write at least 32 bits to RAM atomically.

an easy fix for this would be to allocate the memory for castagnoliTable statically, but this would increase the memory footprint of any program that uses the crc32 package but not the Castagnoli table by 1 kibibyte.

@davecheney
Copy link
Contributor

The race detector reports a data race between these two lines

Thank you for raising this issue. Can you please provide some sample code that exercises this race and the output from the race detector which lead you to raise this issue.

@davecheney davecheney added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Oct 11, 2020
@gopherbot
Copy link

Change https://golang.org/cl/261639 mentions this issue: hash/crc32: fix race between lazy Castagnoli init and Update/Write

@rsc rsc added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 12, 2020
@rsc rsc self-assigned this Oct 12, 2020
@rsc rsc removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Oct 12, 2020
@golang golang locked and limited conversation to collaborators Oct 13, 2021
@rsc rsc removed their assignment Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants