[PATCH] udhcpd: Serve BOOTP clients
This patch makes udhcpd respond correctly to queries from BOOTP clients. It contains the following changes: The end field, or DHCP_END option, is required in DHCP requests but optional in BOOTP requests. Only complain about missing end fields if there are DHCP options in the packet. However, we still send an end field in all replies, because some BOOTP clients expect one in replies even if they didn't send one in the request. Requests without a DHCP_MESSAGE_TYPE are recognized as BOOTP requests and handled appropriately, instead of being discarded. We still require an RFC 1048 options field, but we allow it to be empty. Since a BOOTP client will keep using the assigned IP forever, we only send a BOOTP reply if a static lease exists for that client. BOOTP replies shouldn't contain DHCP_* options, so we omit them if there was no DHCP_MESSAGE_TYPE in the request. Options other than DHCP_* options are still sent. The options field of a BOOTP reply must be exactly 64 bytes. If we construct a reply with more than 64 bytes of options, we give up and log an error instead of sending it. udhcp_send_raw_packet already pads the options field to 64 bytes if it is too short. This implementation has been tested against an HP PA-RISC client. --- networking/udhcp/common.c.orig 2023-05-22 18:41:39.0 -0700 +++ networking/udhcp/common.c 2023-05-21 19:18:15.0 -0700 @@ -234,6 +234,7 @@ void FAST_FUNC init_scan_state(struct dhcp_packet *packet, struct dhcp_scan_state *scan_state) { scan_state->overload = 0; + scan_state->is_dhcp = 0; scan_state->rem = sizeof(packet->options); scan_state->optionptr = packet->options; } @@ -251,12 +252,17 @@ /* option bytes: [code][len][data1][data2]..[dataLEN] */ while (1) { + if (scan_state->rem == 0 && !scan_state->is_dhcp) + break; /* BOOTP packet without end field */ if (scan_state->rem <= 0) { complain: bb_simple_error_msg("bad packet, malformed option field"); return NULL; } + if (scan_state->optionptr[OPT_CODE] >= DHCP_REQUESTED_IP && scan_state->optionptr[OPT_CODE] <= DHCP_CLIENT_ID) + scan_state->is_dhcp = 1; + /* DHCP_PADDING and DHCP_END have no [len] byte */ if (scan_state->optionptr[OPT_CODE] == DHCP_PADDING) { scan_state->rem--; --- networking/udhcp/common.h.orig 2023-01-03 06:17:01.0 -0800 +++ networking/udhcp/common.h 2023-05-21 19:01:24.0 -0700 @@ -123,6 +123,7 @@ struct dhcp_scan_state { int overload; + int is_dhcp; int rem; uint8_t *optionptr; }; --- networking/udhcp/packet.c.orig 2023-01-03 06:17:01.0 -0800 +++ networking/udhcp/packet.c 2023-05-23 00:22:45.0 -0700 @@ -18,6 +18,7 @@ memset(packet, 0, sizeof(*packet)); packet->op = BOOTREQUEST; /* if client to a server */ switch (type) { + case 0: case DHCPOFFER: case DHCPACK: case DHCPNAK: @@ -28,7 +29,8 @@ packet->cookie = htonl(DHCP_MAGIC); if (DHCP_END != 0) packet->options[0] = DHCP_END; - udhcp_add_simple_option(packet, DHCP_MESSAGE_TYPE, type); + if (type) + udhcp_add_simple_option(packet, DHCP_MESSAGE_TYPE, type); } #endif --- networking/udhcp/dhcpd.c.orig 2023-01-03 06:17:01.0 -0800 +++ networking/udhcp/dhcpd.c2023-05-28 17:08:51.0 -0700 @@ -649,7 +649,8 @@ packet->flags = oldpacket->flags; packet->gateway_nip = oldpacket->gateway_nip; packet->ciaddr = oldpacket->ciaddr; - udhcp_add_simple_option(packet, DHCP_SERVER_ID, server_data.server_nip); + if(type) + udhcp_add_simple_option(packet, DHCP_SERVER_ID, server_data.server_nip); } /* Fill options field, siaddr_nip, and sname and boot_file fields. @@ -733,8 +734,11 @@ { struct dhcp_packet packet; uint32_t lease_time_sec; + char message_type = DHCPOFFER; - init_packet(, oldpacket, DHCPOFFER); + if (!udhcp_get_option(oldpacket, DHCP_MESSAGE_TYPE)) + message_type = 0; + init_packet(, oldpacket, message_type); /* If it is a static lease, use its IP */ packet.yiaddr = static_lease_nip; @@ -785,8 +789,13 @@ } lease_time_sec = select_lease_time(oldpacket); - udhcp_add_simple_option(, DHCP_LEASE_TIME, htonl(lease_time_sec)); + if (udhcp_get_option(oldpacket, DHCP_MESSAGE_TYPE)) + udhcp_add_simple_option(, DHCP_LEASE_TIME, htonl(lease_time_sec)); add_server_options(); + if (!udhcp_get_option(oldpacket, DHCP_MESSAGE_TYPE) && udhcp_end_option(packet.options) > 63) { + bb_simple_error_msg("BOOTP BOOTREPLY would be too large, not sending"); +
Re: [PATCH] libbb: in xmalloc_fgetline, use getline if enabled
On Sun, 28 May 2023 19:01:52 +0200 Elvira Khabirova wrote: > When xmalloc_fgetline was introduced, getline(3) was a GNU extension > and was not necessarily implemented in libcs. Since then, > it's become a part of POSIX.1-2008 and is now implemented in > glibc, uClibc-ng, and musl. > > Previously, xmalloc_fgetline directly called bb_get_chunk_from_file. > The issue with that approach is that bb_get_chunk_from_file stops > both at \n and at \0. This introduces unexpected behavior when tools > are presented with inputs containing \0. For example, in a comparison > of GNU core utils cut with busybox cut: > > % echo -ne '\x00\x01\x00\x03\x04\x05\x06' | cut -b1-3 | hexdump -C > 00 01 00 0a || > 0004 > % echo -ne '\x00\x01\x00\x03\x04\x05\x06' | ./busybox cut -b1-3 | hexdump -C > 0a 01 0a 03 04 05 0a |...| > 0007 > > Here, due to bb_get_chunk_from_file splitting lines by \n and \0, > busybox cut interprets the input as three lines instead of one. > > Also, since xmalloc_fgetline never returned strings with embedded \0, > cut_file expects strlen of the returned string to match the string's > total length. > > To fix the behavior of the cut utility, introduce xmalloc_fgetline_n, > that fully matches the behavior of xmalloc_fgetline, > but also returns the amount of bytes read. > > Add a configuration option, FEATURE_USE_GETLINE, and enable it > by default. Use getline in xmalloc_fgetline_n if the feature is enabled. > > Change the behavior of cut_file to use the values returned > by the new function instead of calling strlen. > > Call xmalloc_fgetline_n from xmalloc_fgetline. > > Add a test for the previously mentioned case. > > With FEATURE_USE_GETLINE: > > function old new delta > xmalloc_fgetline_n - 173+173 > xmalloc_fgetline 85 58 -27 > cut_main14061367 -39 > -- > (add/remove: 1/0 grow/shrink: 0/2 up/down: 173/-66) Total: 107 bytes > > Without FEATURE_USE_GETLINE: > > function old new delta > xmalloc_fgetline_n - 41 +41 > xmalloc_fgetline 85 58 -27 > cut_main14061367 -39 > -- > (add/remove: 1/0 grow/shrink: 0/2 up/down: 41/-66)Total: -25 bytes > > Fixes: https://bugs.busybox.net/show_bug.cgi?id=15276 > Signed-off-by: Elvira Khabirova > --- > coreutils/cut.c| 4 ++-- > include/libbb.h| 2 ++ > libbb/Config.src | 11 +++ > libbb/get_line_from_file.c | 37 - > testsuite/cut.tests| 2 ++ > 5 files changed, 49 insertions(+), 7 deletions(-) > > diff --git a/coreutils/cut.c b/coreutils/cut.c > index 55bdd9386..7c87467ca 100644 > --- a/coreutils/cut.c > +++ b/coreutils/cut.c > @@ -89,6 +89,7 @@ static void cut_file(FILE *file, const char *delim, const > char *odelim, > const struct cut_list *cut_lists, unsigned nlists) > { > char *line; > + size_t linelen = 0; > unsigned linenum = 0; /* keep these zero-based to be consistent */ > regex_t reg; > int spos, shoe = option_mask32 & CUT_OPT_REGEX_FLGS; > @@ -96,10 +97,9 @@ static void cut_file(FILE *file, const char *delim, const > char *odelim, > if (shoe) xregcomp(, delim, REG_EXTENDED); > > /* go through every line in the file */ > - while ((line = xmalloc_fgetline(file)) != NULL) { > + while ((line = xmalloc_fgetline_n(file, )) != NULL) { > > /* set up a list so we can keep track of what's been printed */ > - int linelen = strlen(line); > char *printed = xzalloc(linelen + 1); > char *orig_line = line; > unsigned cl_pos = 0; > diff --git a/include/libbb.h b/include/libbb.h > index 6191debb1..73d16647a 100644 > --- a/include/libbb.h > +++ b/include/libbb.h > @@ -1048,6 +1048,8 @@ extern char *xmalloc_fgetline_str(FILE *file, const > char *terminating_string) FA > extern char *xmalloc_fgets(FILE *file) FAST_FUNC RETURNS_MALLOC; > /* Chops off '\n' from the end, unlike fgets: */ > extern char *xmalloc_fgetline(FILE *file) FAST_FUNC RETURNS_MALLOC; > +/* Same as xmalloc_fgetline but returns number of bytes read: */ > +extern char* xmalloc_fgetline_n(FILE *file, size_t *n) FAST_FUNC > RETURNS_MALLOC; > /* Same, but doesn't try to conserve space (may have some slack after the > end) */ > /* extern char *xmalloc_fgetline_fast(FILE *file) FAST_FUNC RETURNS_MALLOC; > */ > >
[PATCH] libbb: in xmalloc_fgetline, use getline if enabled
When xmalloc_fgetline was introduced, getline(3) was a GNU extension and was not necessarily implemented in libcs. Since then, it's become a part of POSIX.1-2008 and is now implemented in glibc, uClibc-ng, and musl. Previously, xmalloc_fgetline directly called bb_get_chunk_from_file. The issue with that approach is that bb_get_chunk_from_file stops both at \n and at \0. This introduces unexpected behavior when tools are presented with inputs containing \0. For example, in a comparison of GNU core utils cut with busybox cut: % echo -ne '\x00\x01\x00\x03\x04\x05\x06' | cut -b1-3 | hexdump -C 00 01 00 0a || 0004 % echo -ne '\x00\x01\x00\x03\x04\x05\x06' | ./busybox cut -b1-3 | hexdump -C 0a 01 0a 03 04 05 0a |...| 0007 Here, due to bb_get_chunk_from_file splitting lines by \n and \0, busybox cut interprets the input as three lines instead of one. Also, since xmalloc_fgetline never returned strings with embedded \0, cut_file expects strlen of the returned string to match the string's total length. To fix the behavior of the cut utility, introduce xmalloc_fgetline_n, that fully matches the behavior of xmalloc_fgetline, but also returns the amount of bytes read. Add a configuration option, FEATURE_USE_GETLINE, and enable it by default. Use getline in xmalloc_fgetline_n if the feature is enabled. Change the behavior of cut_file to use the values returned by the new function instead of calling strlen. Call xmalloc_fgetline_n from xmalloc_fgetline. Add a test for the previously mentioned case. With FEATURE_USE_GETLINE: function old new delta xmalloc_fgetline_n - 173+173 xmalloc_fgetline 85 58 -27 cut_main14061367 -39 -- (add/remove: 1/0 grow/shrink: 0/2 up/down: 173/-66) Total: 107 bytes Without FEATURE_USE_GETLINE: function old new delta xmalloc_fgetline_n - 41 +41 xmalloc_fgetline 85 58 -27 cut_main14061367 -39 -- (add/remove: 1/0 grow/shrink: 0/2 up/down: 41/-66)Total: -25 bytes Fixes: https://bugs.busybox.net/show_bug.cgi?id=15276 Signed-off-by: Elvira Khabirova --- coreutils/cut.c| 4 ++-- include/libbb.h| 2 ++ libbb/Config.src | 11 +++ libbb/get_line_from_file.c | 37 - testsuite/cut.tests| 2 ++ 5 files changed, 49 insertions(+), 7 deletions(-) diff --git a/coreutils/cut.c b/coreutils/cut.c index 55bdd9386..7c87467ca 100644 --- a/coreutils/cut.c +++ b/coreutils/cut.c @@ -89,6 +89,7 @@ static void cut_file(FILE *file, const char *delim, const char *odelim, const struct cut_list *cut_lists, unsigned nlists) { char *line; + size_t linelen = 0; unsigned linenum = 0; /* keep these zero-based to be consistent */ regex_t reg; int spos, shoe = option_mask32 & CUT_OPT_REGEX_FLGS; @@ -96,10 +97,9 @@ static void cut_file(FILE *file, const char *delim, const char *odelim, if (shoe) xregcomp(, delim, REG_EXTENDED); /* go through every line in the file */ - while ((line = xmalloc_fgetline(file)) != NULL) { + while ((line = xmalloc_fgetline_n(file, )) != NULL) { /* set up a list so we can keep track of what's been printed */ - int linelen = strlen(line); char *printed = xzalloc(linelen + 1); char *orig_line = line; unsigned cl_pos = 0; diff --git a/include/libbb.h b/include/libbb.h index 6191debb1..73d16647a 100644 --- a/include/libbb.h +++ b/include/libbb.h @@ -1048,6 +1048,8 @@ extern char *xmalloc_fgetline_str(FILE *file, const char *terminating_string) FA extern char *xmalloc_fgets(FILE *file) FAST_FUNC RETURNS_MALLOC; /* Chops off '\n' from the end, unlike fgets: */ extern char *xmalloc_fgetline(FILE *file) FAST_FUNC RETURNS_MALLOC; +/* Same as xmalloc_fgetline but returns number of bytes read: */ +extern char* xmalloc_fgetline_n(FILE *file, size_t *n) FAST_FUNC RETURNS_MALLOC; /* Same, but doesn't try to conserve space (may have some slack after the end) */ /* extern char *xmalloc_fgetline_fast(FILE *file) FAST_FUNC RETURNS_MALLOC; */ diff --git a/libbb/Config.src b/libbb/Config.src index b980f19a9..7a37929d6 100644 --- a/libbb/Config.src +++ b/libbb/Config.src @@ -147,6 +147,17 @@ config MONOTONIC_SYSCALL will be used instead (which gives wrong results if date/time is