Fix stale goroutine label#1453
Conversation
Previously it was possible that if a label was shorter than a previous one on the same CPU, that we didn't clear the remaining bytes, causing incorrect labels to be reported.
|
For the record this broke when we removed the zeroing here: 40dd9d9 |
| // otherwise inherit the old trailing bytes. Userspace reconstructs the | ||
| // string by scanning for the first NUL, so leftover bytes would corrupt | ||
| // the label keys and values we emit. | ||
| __builtin_memset(lbl, 0, sizeof(*lbl)); |
There was a problem hiding this comment.
memset unrolls to 64 1 byte writes, not the end of the world but instead I think we should just write a null terminator since we don't store the length and the Go side treats it as a C-string. This is why we read at most LEN-1.
| trace := t.tracePool.Get().(*libpf.EbpfTrace) | ||
| // comm is best-effort: if the kernel handed us non-UTF-8 bytes, drop it | ||
| // rather than emit an invalid string downstream. | ||
| comm, _ := goString(ptr.Comm[:]) |
There was a problem hiding this comment.
Doing utf8 validation on comm feels wrong (like wasted energy) but its probably not a big deal.
| return libpf.Intern(pfunsafe.ToString(cstr[:index])) | ||
| b := cstr[:index] | ||
| if !utf8.Valid(b) { | ||
| return libpf.NullString, false |
There was a problem hiding this comment.
This throws away all the valid bits of the string in the case of a mid-rune truncation, feels like a bad idea, wouldn't it be better to skip the utf8 validation and let downstream consumers deal with it?
|
Closing to have @gnurizen take this over the finish line. |
|
super ceded by #1454 |
2 participants
Previously it was possible that if a label was shorter than a previous one on the same CPU, that we didn't clear the remaining bytes, causing incorrect labels to be reported.
@florianl @christos68k