Re: [PATCH v2 5/6] dhcp: Add option overload

On 5/26/26 14:31, Anshu Kumari wrote:

A user can enter lots of options in command-line which may not fit in existing buffer, So when the options field is full, overflow remaining DHCP options into the file and sname fields per RFC 2132 option 52.

Also, when the file field is not used for overload, copy the boot file URL there directly for legacy PXE clients.

This patch not only adds the overload, it also copies the options from the command line (custom_opts) to the actual opts.

Link: https://bugs.passt.top/show_bug.cgi?id=192 Signed-off-by: Anshu Kumari --- v2: - Added #define DHCP_OVERLOAD_FILE and #define DHCP_OVERLOAD_SNAME constants - Added comment documenting space reservation: /* Reserve 3 bytes for option 52 */ - Fixed DNS search length: sizeof(m->o) only, not combined with file+sname - Removed dhcp_boot references — reply.file copy now reads from opts[67] - Used DHCP_OVERLOAD_FILE constant in reply.file guard --- dhcp.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 75 insertions(+), 9 deletions(-)

diff --git a/dhcp.c b/dhcp.c index e126063..a49a05a 100644 --- a/dhcp.c +++ b/dhcp.c @@ -306,14 +306,59 @@ static bool fill_one(uint8_t *buf, size_t size, int o, int *offset) return false; }

+#define DHCP_OVERLOAD_FILE 1 +#define DHCP_OVERLOAD_SNAME 2 + +/** + * fill_overflow() - Fill remaining options into file and sname fields + * @m: Message whose file/sname fields may be used for overflow + * + * Return: option 52 overload value: 0 if no overflow, + * DHCP_OVERLOAD_FILE for file, DHCP_OVERLOAD_SNAME for sname, + * or both OR'd together + */ +static int fill_overflow(struct msg *m) +{ + int file_off = 0, sname_off = 0, overload = 0; + int o; + + for (o = 0; o < 255; o++) { + if (opts[o].slen == -1 || opts[o].sent) + continue; + fill_one(m->file, sizeof(m->file) - 1, o, &file_off); + } + + for (o = 0; o < 255; o++) { + if (opts[o].slen == -1 || opts[o].sent) + continue; + if (fill_one(m->sname, sizeof(m->sname) - 1, o, &sname_off)) + debug("DHCP: skipping option %i (overload full)", o); + } + + if (file_off) { + m->file[file_off] = 255; + overload |= DHCP_OVERLOAD_FILE; + } + + if (sname_off) { + m->sname[sname_off] = 255; + overload |= DHCP_OVERLOAD_SNAME; + } + + return overload; +} + /** - * fill() - Fill options in message + * fill() - Fill options in message, with overload into file/sname if needed * @m: Message to fill + * @overload: Set to option 52 value (0 if none, 1/2/3 per RFC 2132) * * Return: current size of options field */ -static int fill(struct msg *m) +static int fill(struct msg *m, int *overload) { + /* Reserve 3 bytes for option 52 (overload) if needed */ + size_t size = OPT_MAX - 3; int i, o, offset = 0;

for (o = 0; o < 255; o++) @@ -324,20 +369,25 @@ static int fill(struct msg *m) * Put it there explicitly, unless requested via option 55. */ if (opts[55].clen > 0 && !memchr(opts[55].c, 53, opts[55].clen)) - if (fill_one(m->o, OPT_MAX, 53, &offset)) - debug("DHCP: skipping option 53"); + fill_one(m->o, size, 53, &offset);

for (i = 0; i < opts[55].clen; i++) { o = opts[55].c[i]; if (opts[o].slen != -1) - if (fill_one(m->o, OPT_MAX, o, &offset)) - debug("DHCP: skipping option %i", o); + fill_one(m->o, size, o, &offset); }

for (o = 0; o < 255; o++) { if (opts[o].slen != -1 && !opts[o].sent) - if (fill_one(m->o, OPT_MAX, o, &offset)) - debug("DHCP: skipping option %i", o); + fill_one(m->o, size, o, &offset); + } + + *overload = fill_overflow(m); + + if (*overload) { + m->o[offset++] = 52; + m->o[offset++] = 1; + m->o[offset++] = *overload; }

m->o[offset++] = 255; @@ -462,6 +512,7 @@ int dhcp(const struct ctx *c, struct iov_tail *data) struct msg const *m; struct msg reply; unsigned int i; + int overload;

eh = IOV_REMOVE_HEADER(data, eh_storage); iph = IOV_PEEK_HEADER(data, iph_storage); @@ -613,7 +664,22 @@ int dhcp(const struct ctx *c, struct iov_tail *data) if (!c->no_dhcp_dns_search) opt_set_dns_search(c, sizeof(m->o));

opt_set_dns_search() manages option 119 but we have also it in custom_opts (incorrectly set as it doesn't manage compression). I think we overwrite it with incorrect values. Moreover I think this function is broken, because max_len is computed using opts[].slen, but opts[].slen is updated below (and max_len should be OPT_MAX - 3, not sizeof(m->o))

- dlen = offsetof(struct msg, o) + fill(&reply); + for (i = 0; i < (unsigned int)c->custom_opts_count; i++) { + uint8_t code = c->custom_opts[i].code; + + opts[code].slen = c->custom_opts[i].len; + memcpy(opts[code].s, c->custom_opts[i].val, + c->custom_opts[i].len); + } + + dlen = offsetof(struct msg, o) + fill(&reply, &overload); + + /* Copy boot file name into the file field for legacy PXE clients, + * unless the file field is already used for option overload. + */ + if (!(overload & DHCP_OVERLOAD_FILE) && + opts[67].slen > 0 && (size_t)opts[67].slen < sizeof(reply.file)) + memcpy(&reply.file, opts[67].s, opts[67].slen + 1);

No need for "&" in "&reply.size". the "+ 1" is wrong as reply.file has been cleared by a memset() (whereas opts[67].s[slen] can be not NUL).

if (m->flags & FLAG_BROADCAST) dst = in4addr_broadcast;

Thanks, Laurent