Skip to content

bgpd: fix unaligned pointer cast in bgp_attr_cluster_list() #22442

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.

Sign up for GitHub

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

Open
guoguojia2021 wants to merge 1 commit into
base: master
Choose a base branch
from
+4 −4
Open

bgpd: fix unaligned pointer cast in bgp_attr_cluster_list() #22442

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions bgpd/bgp_attr.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2777,11 +2777,11 @@ bgp_attr_cluster_list(struct bgp_attr_parser_args *args)
if (peer->discard_attrs[args->type] || peer->withdraw_attrs[args->type])
goto cluster_list_ignore;

bgp_attr_set_cluster(attr, cluster_parse((struct in_addr *)stream_pnt(connection->curr),
length));
struct in_addr *cluster_buf = XMALLOC(MTYPE_TMP, length);

/* XXX: Fix cluster_parse to use stream API and then remove this */
stream_forward_getp(connection->curr, length);
stream_get(cluster_buf, connection->curr, length);

@greptile-apps greptile-apps Bot Jun 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Consider stream_get2 / STREAM_GET for consistent error handling

stream_get() (void return) calls assert internally on underflow, killing the daemon process rather than returning an error that the BGP attribute parser can handle gracefully. The rest of the BGP attribute parsing code uses the STREAM_GET macro (backed by stream_get2, which returns bool and jumps to stream_failure) to avoid hard asserts on malformed input. While the upstream length validation at line 2770 makes an underflow unlikely here, switching to STREAM_GET with a stream_failure: label would be consistent with the surrounding code and guard against any future refactoring that weakens that pre-check.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: bgpd/bgp_attr.c
Line: 2782

Comment:
**Consider `stream_get2` / `STREAM_GET` for consistent error handling**

`stream_get()` (void return) calls `assert` internally on underflow, killing the daemon process rather than returning an error that the BGP attribute parser can handle gracefully. The rest of the BGP attribute parsing code uses the `STREAM_GET` macro (backed by `stream_get2`, which returns `bool` and jumps to `stream_failure`) to avoid hard asserts on malformed input. While the upstream length validation at line 2770 makes an underflow unlikely here, switching to `STREAM_GET` with a `stream_failure:` label would be consistent with the surrounding code and guard against any future refactoring that weakens that pre-check.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@riw777 riw777 Jun 23, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks valid.

bgp_attr_set_cluster(attr, cluster_parse(cluster_buf, length));
XFREE(MTYPE_TMP, cluster_buf);

return BGP_ATTR_PARSE_PROCEED;

Expand Down
Loading