Re: [PATCH v6 6/6] dhcp: Add RFC 3396 option splitting for concatenation-requiring options

On 8/24/26 15:44, Anshu Kumari wrote:

Implement option splitting per RFC 3396 for options that may exceed 255 bytes. A new concat_req[] lookup table marks options requiring concatenation (currently option 81, Client FQDN per RFC 4702).

The opts[].s buffer is resized from 255 to 497 bytes (OPT_CONCAT_MAX) to hold the maximum data that can be split across the options field, file field, and sname field.

When a concatenation-requiring option does not fit as a single option in any field, fill() calls fill_split() to split it across fields in RFC 3396 order: options field first, then file, then sname.

Link: https://bugs.passt.top/show_bug.cgi?id=192 Signed-off-by: Anshu Kumari --- v6: - Merged v5 patches 6/7 and 7/7 into a single patch. - Replaced DHCP_OPT_STR_CONCAT enum value and is_concat_opt() helper with a concat_req[] boolean lookup table. - Used MIN() macro instead of ternary for chunk size. - Fixed space calculation to account for 2-byte code+length overhead per chunk.

v5: - New patch: implement option splitting per RFC 3396 for options exceeding 255 bytes - Add DHCP_OPT_STR_CONCAT type, is_concat_opt(), fill_split() helpers - Resize opts[].s from 255 to OPT_CONCAT_MAX (497) bytes - Add /* fallthrough */ between DHCP_OPT_STR and DHCP_OPT_STR_CONCAT case --- dhcp.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 98 insertions(+), 1 deletion(-)

diff --git a/dhcp.c b/dhcp.c index 43ce133..eda04d1 100644 --- a/dhcp.c +++ b/dhcp.c @@ -35,6 +35,11 @@ #include "dhcp.h" #include "parse.h"

+/* RFC 3396: maximum option data that can be split across options field, + * file field, and sname field (minus code+length overhead per portion). + */ +#define OPT_CONCAT_MAX 497 + /** * enum opt_state - DHCP option state * @OPT_UNSET: Option not configured @@ -59,7 +64,7 @@ enum opt_state { struct opt { int sent; int slen; - uint8_t s[255]; + uint8_t s[OPT_CONCAT_MAX]; int clen; uint8_t c[255]; enum opt_state state; @@ -215,6 +220,13 @@ static const enum dhcp_opt_type dhcp_opt_types[] = { [252] = DHCP_OPT_STR, /* WPAD URL */ };

+/** + * concat_req - Options requiring RFC 3396 concatenation, indexed by code + */ +static const bool concat_req[256] = { + [81] = true, /* Client FQDN (RFC 4702, Section 2) */ +}; + /** * dhcp_opt_parse() - Parse a DHCP option value * @code: DHCP option code @@ -309,6 +321,9 @@ static int dhcp_opt_parse(uint8_t code, const char *str, case DHCP_OPT_STR: slen = strlen(str);

+ if (!concat_req[code] && slen > 255) + return -1; + if (slen >= buf_len) return -1;

@@ -442,6 +457,40 @@ enum dhcp_overload { DHCP_OVERLOAD_SNAME = 2, };

+/** + * fill_split() - Write a split portion of an option into a buffer + * @buf: Buffer to write into + * @size: Usable size of @buf + * @o: Option number (code) + * @offset: Current offset within @buf, updated on write + * @data: Pointer to remaining option data to write + * @remaining: Bytes of option data still to write + * + * Return: number of data bytes written (excluding code+length header) + */ +static size_t fill_split(uint8_t *buf, size_t size, int o, int *offset, + const uint8_t *data, size_t remaining) +{ + size_t avail, chunk; + + if (*offset + 2 >= (int)size) + return 0; + + avail = size - *offset - 2; + chunk = MIN(remaining, avail); + if (!chunk) + return 0; + + buf[*offset] = o; + buf[*offset + 1] = chunk;

buf[] is uint8_t, but chunk can be > 255: avail can be OPT_MAX - 3 - 2 (302) remainining is, on first call opts[81].slen (that is opts[81].slen = strlen(c->fqdn) + 5, that can be PASST_MAXDNAME - 1 + 5 = 258). So to avoid this overflow (or other in future options) you should use chunk = MIN(MIN(remaining, avail), 255)

+ *offset += 2; + + memcpy(buf + *offset, data, chunk); + *offset += chunk; + + return chunk; +} + /** * fill() - Fill options in message, with overload into file/sname if needed * @m: Message to fill @@ -495,6 +544,54 @@ static int fill(struct msg *m, enum dhcp_overload *overload, bool has_bootfile) } }

+ /* RFC 3396: split concatenation-requiring options that didn't fit + * as a single option. Split order: options, file, sname. + */ + for (o = 0; (size_t)o < ARRAY_SIZE(opts); o++) { + size_t file_cap, sname_cap, total, written; + + if (opts[o].state == OPT_UNSET || opts[o].sent || + !concat_req[o]) + continue; + + sname_cap = sizeof(m->sname) - 1 > (size_t)sname_off ? + sizeof(m->sname) - 1 - sname_off : 0; + + if (has_bootfile || sizeof(m->file) - 1 <= (size_t)file_off) + file_cap = 0; + else + file_cap = sizeof(m->file) - 1 - file_off; + + total = (size > (size_t)offset ? size - offset - 2 : 0)

here also 'size - offset - 2' must be capped by 255 otherwise total available space is not correctly evaluated.

+ + (file_cap > 2 ? file_cap - 2 : 0) + + (sname_cap > 2 ? sname_cap - 2 : 0); + + if (total < (size_t)opts[o].slen) { + debug("DHCP: skipping option %i (no space to split)", + o); + continue; + } + + written = 0; + written += fill_split(m->o, size, o, &offset, + opts[o].s, opts[o].slen); + if (written < (size_t)opts[o].slen && !has_bootfile) + written += fill_split(m->file, + sizeof(m->file) - 1, o, + &file_off, + opts[o].s + written, + opts[o].slen - written); + if (written < (size_t)opts[o].slen) + written += fill_split(m->sname, + sizeof(m->sname) - 1, o, + &sname_off, + opts[o].s + written, + opts[o].slen - written); + + if (written >= (size_t)opts[o].slen) + opts[o].sent = 1; + } + /* Report any options that could not be sent */ for (o = 0; (size_t)o < ARRAY_SIZE(opts); o++) { if (opts[o].state != OPT_UNSET && !opts[o].sent)