Skip to content

init/dhcp: only overwrite resolv.conf with DNS #698

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
slp wants to merge 1 commit into
base: main
Choose a base branch
from
Open

init/dhcp: only overwrite resolv.conf with DNS #698

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
16 changes: 11 additions & 5 deletions init/dhcp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,9 @@ static unsigned char get_dhcp_msg_type(const unsigned char *response,
static int handle_dhcp_ack(int nl_sock, int iface_index,
const unsigned char *response, ssize_t len)
{
FILE *resolv = NULL;
bool tried_opening_resolv = false;
Comment on lines +296 to +297
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 29, 2026

Choose a reason for hiding this comment

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

high

Since resolv is now initialized to NULL and only opened conditionally when a DNS option is present, any subsequent call to fclose(resolv) at the end of handle_dhcp_ack must be guarded with a NULL check (i.e., if (resolv) fclose(resolv);) to prevent a segmentation fault when no DNS options are provided in the DHCP response.

Additionally, if handle_dhcp_ack contains any early return paths after the options loop, resolv must be closed before returning to prevent a file descriptor leak.


/* Need at least 240 bytes (DHCP header + magic cookie) + 1 for options */
if (len < 241) {
printf("DHCPACK too short (%zd bytes)\n", len);
Expand All @@ -314,11 +317,6 @@ static int handle_dhcp_ack(int nl_sock, int iface_index,
/* Clamp MTU to passt's limit */
uint16_t mtu = 65520;

FILE *resolv = fopen("/etc/resolv.conf", "w");
if (!resolv) {
perror("Failed to open /etc/resolv.conf");
}

/* Parse DHCP options (start at offset 240 after magic cookie) */
size_t p = 240;
while (p < (size_t)len) {
Expand Down Expand Up @@ -353,6 +351,14 @@ static int handle_dhcp_ack(int nl_sock, int iface_index,
memcpy(&router.s_addr, &response[p], sizeof(router.s_addr));
} else if (opt == 6 && opt_len >= 4) {
/* Option 6: Domain Name Server */
if (!resolv && !tried_opening_resolv) {
tried_opening_resolv = true;
resolv = fopen("/etc/resolv.conf", "w");
if (!resolv) {
perror("Failed to open /etc/resolv.conf");
}
}
Comment on lines +354 to +360
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 29, 2026

Choose a reason for hiding this comment

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

medium

When fopen fails to open /etc/resolv.conf, the error is silently ignored. We should report this error, but we must prioritize consistency with the existing error reporting patterns in this file (e.g., using stdout vs stderr or a specific logging function) to maintain a uniform codebase, rather than automatically using perror.

            if (!resolv && !tried_opening_resolv) {
                tried_opening_resolv = true;
                resolv = fopen("/etc/resolv.conf", "w");
                if (!resolv) {
                    // Use the consistent error reporting pattern of this file
                }
            }
References
  1. Prioritize consistency with existing error reporting patterns in a file (e.g., using stdout vs stderr) over standard external practices to maintain a uniform codebase.

Comment on lines +354 to +360
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 29, 2026

Choose a reason for hiding this comment

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

medium

To simplify resource management and eliminate function-level state, consider localizing the file opening and closing directly within the DNS option block. Additionally, avoid using the magic number 6 directly; define it as a named constant (e.g., DHCP_OPTION_DNS) for improved readability and maintainability. Since all DNS servers are typically provided within a single option block, you can open /etc/resolv.conf, write the nameservers, and close it immediately within this block. This avoids the need for the function-level resolv and tried_opening_resolv variables, and completely eliminates the risk of leaks on early returns or crashes from fclose(NULL).

References
  1. When using magic numbers for DHCP options, define them as named constants for improved readability and maintainability.


if (resolv) {
for (int dns_p = p; dns_p + 4 <= p + opt_len; dns_p += 4) {
fprintf(resolv, "nameserver %d.%d.%d.%d\n", response[dns_p],
Expand Down
Loading