Skip to content

pmproxy/UBSAN: fix access to misaligned address 0x000000000007 - #2750

Open
rmetrich wants to merge 1 commit into
performancecopilot:mainfrom
rmetrich:ubsan__3
Open

rmetrich wants to merge 1 commit into
performancecopilot:mainfrom
rmetrich:ubsan__3

Conversation

@rmetrich

@rmetrich rmetrich commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

Pull Request Description

Error being reported by UBSAN:

query.c:98:11: runtime error: member access within misaligned address 0x000000000007 for type 'struct node_t', which requires 8 byte alignment 0x000000000007: note: pointer points here
<memory cannot be printed>
    #0 0x7f61e117e896 in skip_free_value_set [...]/src/libpcp_web/src/query.c:98
    #1 0x7f61e117eb1f in freeSeriesQueryNode [...]/src/libpcp_web/src/query.c:114
    #2 0x7f61e1180926 in freeSeriesGetQuery [...]/src/libpcp_web/src/query.c:154
    #3 0x7f61e11822f9 in series_query_finished [...]/src/libpcp_web/src/query.c:284
    #4 0x7f61e11827dc in series_query_end_phase [...]/src/libpcp_web/src/query.c:303
    #5 0x7f61e120551e in key_lookup_mapping_callback [...]/src/libpcp_web/src/query.c:6138
    #6 0x7f61e1248eac in keySlotsReplyCallback [...]/src/libpcp_web/src/slots.c:590
    #7 0x7f61e12d95a3 in valkeyClusterAsyncCallback deps/libvalkey/src/cluster.c:3051
    #8 0x7f61e12c1473 in valkeyRunCallback deps/libvalkey/src/async.c:300
    #9 0x7f61e12c219c in valkeyAsyncFreeInternal deps/libvalkey/src/async.c:349
    #10 0x7f61e1247db4 in valkeyLibuvPoll ../../../vendor/github.com/valkey-io/libvalkey/include/valkey/adapters/libuv.h:54
    #11 0x7f61e1d8154c in uv__io_poll /usr/src/debug/libuv-1.52.1-1.el10_2.x86_64/src/unix/linux.c:1546
    #12 0x7f61e1d6b9f1 in uv_run /usr/src/debug/libuv-1.52.1-1.el10_2.x86_64/src/unix/core.c:460
    #13 0x55c5271c9217 in main_loop [...]/src/pmproxy/src/server.c:921
    #14 0x55c5271c823f in main [...]/src/pmproxy/src/pmproxy.c:473
    #15 0x7f61dfe5158d in __libc_start_call_main (/lib64/libc.so.6+0x2a58d) (BuildId: cc6959d636202b0e1d6e17b273bf4060ec9265e5)
    #16 0x7f61dfe51648 in __libc_start_main_alias_2 (/lib64/libc.so.6+0x2a648) (BuildId: cc6959d636202b0e1d6e17b273bf4060ec9265e5)
    #17 0x55c5271c8c34 in _start ([...]/src/pmproxy/src/pmproxy+0x65c34) (BuildId: 8145839e69ef9e1eeb443adc33bf70cf083cf8ba)

Root cause:

seriesGetLookup (which contains seriesGetSID series[], a flexible array member) was placed before seriesGetQuery query in seriesQueryBaton structure:

typedef struct seriesQueryBaton {
    seriesBatonMagic    header;         /* MAGIC_QUERY */
    seriesBatonPhase    *current;
    seriesBatonPhase    phases[QUERY_PHASES];
    pmSeriesModule      *module;
    pmSeriesCallBacks   *callbacks;
    pmLogInfoCallBack   info;
    void                *userdata;
    keySlots            *slots;
    int                 error;
    seriesGetLookup     lookup;
    seriesGetQuery      query;
} seriesQueryBaton;

This cause overwriting baton->query.root with sid->header.magic = 7 when calling initSeriesBatonMagic(sid, MAGIC_SID).

Trigger:

When the valkey connection dropped, the error path in series_query_end_phase called series_query_finished → freeSeriesGetQuery on a lookup baton, which then passed the corrupted SeriesBaton magic (0x7) to be passed to freeSeriesQueryNode.

Fix:

Because seriesGetLookup is a flexible array, it has to be last in structure, therefore seriesGetQuery and seriesGetLookup must be swapped.

Reproducer:

In grafana open the PCP Valkey: Metric Preview, set to 5s refresh, then continuously start/stop the valkey service:

  # while :; do systemctl stop valkey; echo "valkey stopped"; sleep 3.1; \
    systemctl start valkey; echo "valkey started"; sleep 10.1; done

After some time (unpredictable), the connection may break just after a Lookup Baton has been referenced, causing freeSeriesGetQuery to be freed instead. This was verified with PCP_DEBUG=desperate level and new instrumentation on line 218; pmproxy.log then shows the issue on Baton 0x51500007c680:

Baton [query/0x51500007c680] verified magic - 6 (@ series_lookup_services)
Baton [query/0x51500007c680] references: 0 -> 1 (@ series_lookup_services)
Baton [query/0x51500007c680] verified magic - 6 (@ series_query_end_phase)
Baton [query/0x51500007c680] references: 1 -> 0 (@ series_query_end_phase[seriesPassBaton])
Baton [query/0x51500007c680] verified magic - 6 (@ series_lookup_mapping)
Baton [query/0x51500007c680] 0 references - verified (@ series_lookup_mapping)

HERE BELOW Lookup Baton is referenced
Baton [query/0x51500007c680] references: 0 -> 1 (@ series_lookup_mapping)

 :
[Fri Oct  2 13:13:28] pmproxy(375872) Error: Lost connection to key server.
 :

Baton [query/0x51500007c680] verified magic - 6 (@ key_lookup_mapping_callback)
Baton [query/0x51500007c680] verified magic - 6 (@ series_query_end_phase)

HERE BELOW Query Baton is dereferenced while it should be Lookup Baton!!
Baton [query/0x51500007c680] references: 1 -> 0 (@ series_query_end_phase)
Baton [query/0x51500007c680] verified magic - 6 (@ freeSeriesGetQuery)
Baton [query/0x51500007c680] 0 references - verified (@ freeSeriesGetQuery)
query.c:98:11: runtime error: member access within misaligned address 0x000000000007 for type 'struct node_t', which requires 8 byte alignment [...]

Related PR

See also ASAN+UBSAN PR

@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration
  • Configuration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited)
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 24273cfa-8bbf-4739-8269-27dbd96b695e

📥 Commits

Reviewing files that changed from the base of the PR and between 9334540 and cefb8dd.


You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited)
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 8fa021d7-71f9-48fc-8136-5bc06191e0da

📥 Commits

Reviewing files that changed from the base of the PR and between f042c2e and 9334540.


📒 Files selected for processing (1)
  • src/libpcp_web/src/query.c

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review.



📝 Summary

Summary by CodeRabbit

  • Diagnostics
    • Expanded diagnostic tracing when detailed debugging is enabled, providing additional information for investigating query behavior. This change affects diagnostic output only; it does not alter the app’s user-facing features or normal query results.

Walkthrough

The series query baton now places lookup after query, keeping its flexible array last. Initialization now enables baton tracing when pmDebugOptions.desperate is set.

Changes

Series query baton

Layer / File(s) Summary
Baton layout and tracing
src/libpcp_web/src/query.c
seriesQueryBaton now places lookup last. initSeriesQueryBaton sets the traced state from pmDebugOptions.desperate.

Suggested reviewers: natoscott


Priority: ➖ Normal

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 93345

The baton layout and tracing changes show no introduced merge-blocking risk.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the UBSAN misaligned-address fix in pmproxy, which matches the main change.
Description check ✅ Passed The description explains the misaligned-address error, its cause, the structure-member reorder, and a reproducer. It is directly related to the changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.


  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

A rabbit checks the baton’s shape
The lookup rests at array’s end
A tracing switch wakes when set
Two small edits complete the flow
The rabbit hops away to review

Comment @coderabbitai help to get the list of available commands.

Error being reported by UBSAN:

query.c:98:11: runtime error: member access within misaligned address 0x000000000007 for type 'struct node_t', which requires 8 byte alignment
0x000000000007: note: pointer points here
<memory cannot be printed>
    #0 0x7f61e117e896 in skip_free_value_set [...]/src/libpcp_web/src/query.c:98
    performancecopilot#1 0x7f61e117eb1f in freeSeriesQueryNode [...]/src/libpcp_web/src/query.c:114
    performancecopilot#2 0x7f61e1180926 in freeSeriesGetQuery [...]/src/libpcp_web/src/query.c:154
    performancecopilot#3 0x7f61e11822f9 in series_query_finished [...]/src/libpcp_web/src/query.c:284
    performancecopilot#4 0x7f61e11827dc in series_query_end_phase [...]/src/libpcp_web/src/query.c:303
    performancecopilot#5 0x7f61e120551e in key_lookup_mapping_callback [...]/src/libpcp_web/src/query.c:6138
    performancecopilot#6 0x7f61e1248eac in keySlotsReplyCallback [...]/src/libpcp_web/src/slots.c:590
    performancecopilot#7 0x7f61e12d95a3 in valkeyClusterAsyncCallback deps/libvalkey/src/cluster.c:3051
    performancecopilot#8 0x7f61e12c1473 in valkeyRunCallback deps/libvalkey/src/async.c:300
    performancecopilot#9 0x7f61e12c219c in valkeyAsyncFreeInternal deps/libvalkey/src/async.c:349
    performancecopilot#10 0x7f61e1247db4 in valkeyLibuvPoll ../../../vendor/github.com/valkey-io/libvalkey/include/valkey/adapters/libuv.h:54
    performancecopilot#11 0x7f61e1d8154c in uv__io_poll /usr/src/debug/libuv-1.52.1-1.el10_2.x86_64/src/unix/linux.c:1546
    performancecopilot#12 0x7f61e1d6b9f1 in uv_run /usr/src/debug/libuv-1.52.1-1.el10_2.x86_64/src/unix/core.c:460
    performancecopilot#13 0x55c5271c9217 in main_loop [...]/src/pmproxy/src/server.c:921
    performancecopilot#14 0x55c5271c823f in main [...]/src/pmproxy/src/pmproxy.c:473
    performancecopilot#15 0x7f61dfe5158d in __libc_start_call_main (/lib64/libc.so.6+0x2a58d) (BuildId: cc6959d636202b0e1d6e17b273bf4060ec9265e5)
    performancecopilot#16 0x7f61dfe51648 in __libc_start_main_alias_2 (/lib64/libc.so.6+0x2a648) (BuildId: cc6959d636202b0e1d6e17b273bf4060ec9265e5)
    performancecopilot#17 0x55c5271c8c34 in _start ([...]/src/pmproxy/src/pmproxy+0x65c34) (BuildId: 8145839e69ef9e1eeb443adc33bf70cf083cf8ba)

Root cause:

seriesGetLookup (which contains seriesGetSID series[], a flexible array
member) was placed before seriesGetQuery query in seriesQueryBaton:

typedef struct seriesQueryBaton {
    seriesBatonMagic    header;         /* MAGIC_QUERY */
    seriesBatonPhase    *current;
    seriesBatonPhase    phases[QUERY_PHASES];
    pmSeriesModule      *module;
    pmSeriesCallBacks   *callbacks;
    pmLogInfoCallBack   info;
    void                *userdata;
    keySlots            *slots;
    int                 error;
    seriesGetLookup     lookup;
    seriesGetQuery      query;
} seriesQueryBaton;

This cause overwriting baton->query.root with sid->header.magic = 7 when
calling initSeriesBatonMagic(sid, MAGIC_SID).

Trigger:

When the valkey connection dropped, the error path in
series_query_end_phase called series_query_finished → freeSeriesGetQuery
on a lookup baton, which then passed the corrupted SeriesBaton magic
(0x7) to be passed to freeSeriesQueryNode.

Fix:

Because seriesGetLookup is a flexible array, it has to be last in
structure, therefore seriesGetQuery and seriesGetLookup must be swapped.

Reproducer:

In grafana open the PCP Valkey: Metric Preview, set to 5s refresh, then
continuously start/stop the valkey service:

  # while :; do systemctl stop valkey; echo "valkey stopped"; sleep 3.1; \
    systemctl start valkey; echo "valkey started"; sleep 10.1; done

After some time (unpredictable), the connection may break just after a
Lookup Baton has been referenced, causing freeSeriesGetQuery to be freed
instead. This was verified with PCP_DEBUG=desperate level and new
instrumentation on line 218; pmproxy.log then shows the issue on Baton
0x51500007c680:

Baton [query/0x51500007c680] verified magic - 6 (@ series_lookup_services)
Baton [query/0x51500007c680] references: 0 -> 1 (@ series_lookup_services)
Baton [query/0x51500007c680] verified magic - 6 (@ series_query_end_phase)
Baton [query/0x51500007c680] references: 1 -> 0 (@ series_query_end_phase[seriesPassBaton])
Baton [query/0x51500007c680] verified magic - 6 (@ series_lookup_mapping)
Baton [query/0x51500007c680] 0 references - verified (@ series_lookup_mapping)

HERE BELOW Lookup Baton is referenced
Baton [query/0x51500007c680] references: 0 -> 1 (@ series_lookup_mapping)

 :
[Fri Oct  2 13:13:28] pmproxy(375872) Error: Lost connection to key server.
 :

Baton [query/0x51500007c680] verified magic - 6 (@ key_lookup_mapping_callback)
Baton [query/0x51500007c680] verified magic - 6 (@ series_query_end_phase)

HERE BELOW Query Baton is dereferenced while it should be Lookup Baton!!
Baton [query/0x51500007c680] references: 1 -> 0 (@ series_query_end_phase)
Baton [query/0x51500007c680] verified magic - 6 (@ freeSeriesGetQuery)
Baton [query/0x51500007c680] 0 references - verified (@ freeSeriesGetQuery)
query.c:98:11: runtime error: member access within misaligned address 0x000000000007 for type 'struct node_t', which requires 8 byte alignment
[...]

Signed-off-by: Renaud Métrich <rmetrich@redhat.com>
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.

1 participant