Re: [PATCH ulogd2 2/6] db, IP2BIN: correct `format_ipv6()` output buffer sizes

[Jeremy Sowden <jeremy@azazel.net>]

[-- Attachment #1: Type: text/plain, Size: 1659 bytes --]

On 2025-04-23, at 13:22:04 +0200, Florian Westphal wrote:
> Jeremy Sowden <jeremy@azazel.net> wrote:
> > `format_ipv6()` formats IPv6 addresses as hex-strings.  However, sizing for the
> > output buffer is not done quite right.
> > 
> > `format_ipv6()` itself uses the size of `struct in6_addr` to verify that the
> > buffer size is large enough, and the output buffer for the call in util/db.c is
> > sized the same way.  However, the size that should be used is that of the
> > `s6_addr` member of `struct in6_addr`, not that of the whole structure.
> 
> ?
> 
> In what uinverse is sizeof(struct in6_addr) different from
> sizeof(((struct in6_addr) {}).s6_addr)?

A POSIX-compliant one? :)

	The <netinet/in.h> header shall define the in6_addr structure, which shall include at least the following member:

	  uint8_t s6_addr[16]

I dare say it's hair-splitting, but it's the size of the `s6_addr` member that
is significant, not the structure as a whole.

> > The elements of the `ipbin_array` array in ulogd_filter_IP2BIN.c are sized using
> > a local macro, `IPADDR_LENGTH`, which is defined as 128, the number of bits in
> > an IPv6 address; this is much larger than necessary.
> 
> Agreed.
> 
> > +#define FORMAT_IPV6_BUFSZ (2 + sizeof(((struct in6_addr) {}).s6_addr) * 2 + 1)
> 
> I'd prefer to not use the .s6_addr, its not needed.
> 
> > -		for (unsigned j = 0; i < sizeof(*ipv6); j += 4, i += 8) {
> > +		for (unsigned j = 0; i < sizeof(ipv6->s6_addr); j += 4, i += 8) {
> 
> I would leave this as-is.

Fair enough.

> First patch looks good, I'll apply it later today.  Still reviewing the
> rest.

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]