Skip to content

Backport BOOTP from systemd/main (v257-8) to debian/systemd v252 Bookworm .deb#1

Open
avramd wants to merge 5 commits intoagd-debian/252.31-1_deb12u1-basefrom
agd-backport-bootp
Open

Backport BOOTP from systemd/main (v257-8) to debian/systemd v252 Bookworm .deb#1
avramd wants to merge 5 commits intoagd-debian/252.31-1_deb12u1-basefrom
agd-backport-bootp

Conversation

@avramd
Copy link
Copy Markdown
Owner

@avramd avramd commented Jan 7, 2025

Note, the changes in sd-dhcp-client.c constitute a wholesale reimplementation of the original change, not simply a merge or an automated patch. This was necessary because the code in that file had been heavily refactored between systemd v252 and v257.

This update is not necessarily final, since the feature has not yet been merged to systemd/main.

avramd added a commit that referenced this pull request Jan 9, 2025
Copy link
Copy Markdown
Owner Author

@avramd avramd left a comment

Choose a reason for hiding this comment

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

Annotating two bugs

Comment thread src/libsystemd-network/sd-dhcp-client.c Outdated
offer->siaddr,
lease->server_address);

lease->server_address = offer->siaddr || lease->server_address;
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Suggested change
lease->server_address = offer->siaddr || lease->server_address;
lease->server_address = offer->siaddr ? offer->siaddr : lease->server_address;

|| in C returns 0 or 1, not any of its operands

Comment thread src/libsystemd-network/sd-dhcp-client.c Outdated
client->client_id_len);
// For BOOTP packets, we already did all this when processing it as an offer
if (client->bootp)
lease = client->lease;
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

This is the beginning of the bad gateway bug. I'm reusing the existing lease data structure that's a member of the client structure. By itself this isn't bad.

But look at lines 1666-1673 (not part of the diff, you'll have to expand). It discards client->lease and replaces it with lease. If they are the same memory location, then it's bad 🤣

@avramd avramd force-pushed the agd-backport-bootp branch 2 times, most recently from c19b838 to a530067 Compare January 15, 2025 18:36
@avramd avramd force-pushed the agd-backport-bootp branch from a530067 to c78889b Compare January 19, 2025 23:47
@avramd avramd changed the title Backport bootp Backport BOOTP from systemd/main (v257-8) to debian/systemd v252 Bookworm .deb May 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant