dhcpcd: size the escaped-SSID buffers for the worst case by akiernan · Pull Request #727 · NetworkConfiguration/dhcpcd

akiernan · 2026-09-08T09:59:04Z

print_string() renders each non-printable byte as \NNN - four characters plus terminating NUL, failing with ENOBUFS if there is no room for it. An SSID is up to IF_SSIDLEN (32) bytes, so an escaped SSID needs up to (IF_SSIDLEN * 4) + 1 = 129 bytes.

dhcpcd_selectprofile() uses PROFILE_LEN, which is 64 causing:

dhcpcd_selectprofile: No buffer space available

in the log, the resulting call to read_config() then gets an an empty SSID, so no profile ssid ... block matches.

dhcpcd_reportssid() uses IF_SSIDLEN * 4, which is 128 and correct except for the NUL, so it fails only on a 32-byte SSID whose every byte escapes. It then logs an error instead of the "connected to Access Point" line.

Closes #726

coderabbitai · 2026-09-08T09:59:44Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 99e87d7a-4746-4cae-bea9-84426c8765a0

📥 Commits

Reviewing files that changed from the base of the PR and between 678e6e4 and 528e982.

📒 Files selected for processing (1)

src/dhcpcd.h

Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review.

Walkthrough

The change defines IF_SSIDSTRLEN for escaped SSIDs and applies it to SSID formatting, leasefile, DHCP state, DHCPv6 state, and environment-generation buffers.

Changes

Escaped SSID buffer sizing

Layer / File(s)	Summary
Define the escaped SSID length contract `src/dhcpcd.h`, `src/dhcp.h`, `src/dhcp6.h`	`IF_SSIDSTRLEN` includes space for four characters per SSID byte and the terminating NUL byte. DHCP and DHCPv6 leasefile fields use the constant.
Size SSID formatting buffers `src/dhcpcd.c`	`dhcpcd_selectprofile` and `dhcpcd_reportssid` use `IF_SSIDSTRLEN` for escaped SSID buffers.
Apply sizing to output buffers `src/dhcp-common.c`, `src/script.c`	Leasefile and `ifssid` environment-generation buffers use `IF_SSIDSTRLEN`.

Priority: ➖ Normal

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: rsmarples

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 6 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: resizing escaped-SSID buffers for worst-case output.
Description check	✅ Passed	The description accurately explains the escaped-SSID sizing issue, its impact on profile matching and logging, and the intended fix.
Linked Issues check	✅ Passed	The changes satisfy issue `#726`. `IF_SSIDSTRLEN` defines `((IF_SSIDLEN * 4) + 1)`. `dhcpcd_selectprofile()` and `dhcpcd_reportssid()` use this size. The size supports worst-case escaped SSIDs and the t…
Out of Scope Changes check	✅ Passed	The changes replace duplicated escaped-SSID buffer sizes with `IF_SSIDSTRLEN`. The lease-file and script-environment updates protect buffers that use the same escaped SSID representation. These change…

Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

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

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

ColinMcInnes

I agree, no security hole here, just a truncated log, and this should resolve that.

rsmarples

This looks like a good improvement, but what do you think of my suggestion?

rsmarples · 2026-09-11T09:37:24Z

 {
 	struct if_options *ifo;
-	char pssid[PROFILE_LEN];
+	char pssid[(IF_SSIDLEN * 4) + 1];


Why not do this in dhcpcd.h

#define IF_SSIDSTRLEN ((IF_SSIDLEN * 4) + 1) #define PROFILE_LEN IF_SSIDSTRLEN

Then you could re-use IF_SSIDSTRLEN in dhcpcd_reportssid

Unless I'm missing something PROFILE_LEN doesn't need to be the same size as an expanded SSID? Though clearly IF_SSIDSTRLEN is a better answer and I found what looked like three more place where it would be used (PR updated).

We could break it out to make a specific PSSID len. But either way it should be defined in dhcpcd.h instead of magic math'd in the function.

rsmarples

This now looks good, just fix the formatting please.
You can run make format with clang-format v21 installed to fix.

print_string() renders each non-printable byte as \NNN - four characters plus terminating NUL, failing with ENOBUFS if there is no room for it. An SSID is up to IF_SSIDLEN (32) bytes, so an escaped SSID needs up to (IF_SSIDLEN * 4) + 1 = 129 bytes. Add IF_SSIDSTRLEN for this and use it everywhere an escaped SSID is stored. dhcpcd_selectprofile() used PROFILE_LEN, which is 64 causing: dhcpcd_selectprofile: No buffer space available in the log, the resulting call to read_config() then gets an empty SSID, so no `ssid ...` block matches. dhcpcd_reportssid() and make_env() used IF_SSIDLEN * 4, which is 128 and correct except for the NUL, so they fail only on a 32-byte SSID whose every byte escapes. The former then logs an error instead of the "connected to Access Point" line and the latter omits ifssid from the hook environment. dhcp_set_leasefile() and the lease file name buffers were already sized correctly, so switching them is only for consistency. Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>

rsmarples · 2026-09-19T18:41:41Z

Thanks!

ColinMcInnes approved these changes Sep 8, 2026

View reviewed changes

rsmarples reviewed Sep 11, 2026

View reviewed changes

akiernan force-pushed the dhcpcd-size-the-escaped-ssid-buffers branch from 9ae8b19 to 678e6e4 Compare September 12, 2026 18:41

rsmarples requested changes Sep 18, 2026

View reviewed changes

akiernan force-pushed the dhcpcd-size-the-escaped-ssid-buffers branch from 678e6e4 to 528e982 Compare September 19, 2026 11:11

akiernan requested a review from rsmarples September 19, 2026 11:47

ColinMcInnes approved these changes Sep 19, 2026

View reviewed changes

rsmarples merged commit 5a91691 into NetworkConfiguration:master Sep 19, 2026
18 checks passed

Navigation Menu

dhcpcd: size the escaped-SSID buffers for the worst case - #727

dhcpcd: size the escaped-SSID buffers for the worst case#727
rsmarples merged 1 commit into
NetworkConfiguration:masterfrom
akiernan:dhcpcd-size-the-escaped-ssid-buffers

akiernan commented Sep 8, 2026

Uh oh!

coderabbitai Bot commented Sep 8, 2026 •

edited

Loading

❌ Failed checks (1 warning)

Uh oh!

ColinMcInnes left a comment

Uh oh!

rsmarples left a comment

Uh oh!

rsmarples Sep 11, 2026

Uh oh!

akiernan Sep 12, 2026

Uh oh!

ColinMcInnes Sep 13, 2026

Uh oh!

rsmarples left a comment

Uh oh!

Uh oh!

rsmarples commented Sep 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

akiernan commented Sep 8, 2026

Uh oh!

coderabbitai Bot commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

ColinMcInnes left a comment

Choose a reason for hiding this comment

Uh oh!

rsmarples left a comment

Choose a reason for hiding this comment

Uh oh!

rsmarples Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

akiernan Sep 12, 2026

Choose a reason for hiding this comment

Uh oh!

ColinMcInnes Sep 13, 2026

Choose a reason for hiding this comment

Uh oh!

rsmarples left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rsmarples commented Sep 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coderabbitai Bot commented Sep 8, 2026 •

edited

Loading