Otherwise we'll limit messages to 577 bytes, instead of 576 bytes as intended: $ fqdn="thirtytwocharactersforeachlabel.thirtytwocharactersforeachlabel.thirtytwocharactersforeachlabel.thirtytwocharactersforeachlabel.thirtytwocharactersforeachlabel.thirtytwocharactersforeachlabel.thirtytwocharactersforeachlabel.then_make_it_251_with_this" $ hostname="__eighteen_bytes__" $ ./pasta --fqdn ${fqdn} -H ${hostname} -p dhcp.pcap -- /sbin/dhclient -4 Saving packet capture to dhcp.pcap $ tshark -r dhcp.pcap -V -Y 'dhcp.option.value == 5' | grep "Total Length" Total Length: 577 This was hidden by the issue fixed by commit bcc4908c2b4a ("dhcp Remove option 255 length byte") until now. Fixes: 31e8109a86ee ("dhcp, dhcpv6: Add hostname and client fqdn ops") Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- dhcp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dhcp.c b/dhcp.c index 4a209f1..b7d5ea3 100644 --- a/dhcp.c +++ b/dhcp.c @@ -143,7 +143,7 @@ static bool fill_one(struct msg *m, int o, int *offset) size_t slen = opts[o].slen; /* If we don't have space to write the option, then just skip */ - if (*offset + 1 /* length of option */ + slen > OPT_MAX) + if (*offset + 2 /* code and length of option */ + slen > OPT_MAX) return true; m->o[*offset] = o; -- 2.43.0
On Tue, Feb 18, 2025 at 09:52:31AM +0100, Stefano Brivio wrote:Otherwise we'll limit messages to 577 bytes, instead of 576 bytes as intended: $ fqdn="thirtytwocharactersforeachlabel.thirtytwocharactersforeachlabel.thirtytwocharactersforeachlabel.thirtytwocharactersforeachlabel.thirtytwocharactersforeachlabel.thirtytwocharactersforeachlabel.thirtytwocharactersforeachlabel.then_make_it_251_with_this" $ hostname="__eighteen_bytes__" $ ./pasta --fqdn ${fqdn} -H ${hostname} -p dhcp.pcap -- /sbin/dhclient -4 Saving packet capture to dhcp.pcap $ tshark -r dhcp.pcap -V -Y 'dhcp.option.value == 5' | grep "Total Length" Total Length: 577 This was hidden by the issue fixed by commit bcc4908c2b4a ("dhcp Remove option 255 length byte") until now. Fixes: 31e8109a86ee ("dhcp, dhcpv6: Add hostname and client fqdn ops") Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>Reviewed-by: David Gibson <david(a)gibson.dropbear.id.au>--- dhcp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dhcp.c b/dhcp.c index 4a209f1..b7d5ea3 100644 --- a/dhcp.c +++ b/dhcp.c @@ -143,7 +143,7 @@ static bool fill_one(struct msg *m, int o, int *offset) size_t slen = opts[o].slen; /* If we don't have space to write the option, then just skip */ - if (*offset + 1 /* length of option */ + slen > OPT_MAX) + if (*offset + 2 /* code and length of option */ + slen > OPT_MAX) return true; m->o[*offset] = o;-- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson
On Wed, Feb 19, 2025 at 12:21 AM David Gibson <david(a)gibson.dropbear.id.au> wrote:On Tue, Feb 18, 2025 at 09:52:31AM +0100, Stefano Brivio wrote:I see "+ 2" better since it is explicit than the alternative of ">= OPT_MAX", so LGTM. Reviewed-by: Enrique Llorente <ellorent(a)redhat.com> -- Quique Llorente CNV networking Senior Software Engineer Red Hat EMEA ellorent(a)redhat.com @RedHat Red Hat Red HatOtherwise we'll limit messages to 577 bytes, instead of 576 bytes as intended: $ fqdn="thirtytwocharactersforeachlabel.thirtytwocharactersforeachlabel.thirtytwocharactersforeachlabel.thirtytwocharactersforeachlabel.thirtytwocharactersforeachlabel.thirtytwocharactersforeachlabel.thirtytwocharactersforeachlabel.then_make_it_251_with_this" $ hostname="__eighteen_bytes__" $ ./pasta --fqdn ${fqdn} -H ${hostname} -p dhcp.pcap -- /sbin/dhclient -4 Saving packet capture to dhcp.pcap $ tshark -r dhcp.pcap -V -Y 'dhcp.option.value == 5' | grep "Total Length" Total Length: 577 This was hidden by the issue fixed by commit bcc4908c2b4a ("dhcp Remove option 255 length byte") until now. Fixes: 31e8109a86ee ("dhcp, dhcpv6: Add hostname and client fqdn ops") Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com>Reviewed-by: David Gibson <david(a)gibson.dropbear.id.au>--- dhcp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dhcp.c b/dhcp.c index 4a209f1..b7d5ea3 100644 --- a/dhcp.c +++ b/dhcp.c @@ -143,7 +143,7 @@ static bool fill_one(struct msg *m, int o, int *offset) size_t slen = opts[o].slen; /* If we don't have space to write the option, then just skip */ - if (*offset + 1 /* length of option */ + slen > OPT_MAX) + if (*offset + 2 /* code and length of option */ + slen > OPT_MAX) return true; m->o[*offset] = o;-- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson