Bug in IN6_IS_ADDR_V4COMPAT macro for addresses ending in .1

Bug in IN6_IS_ADDR_V4COMPAT macro for addresses ending in .1

[Yusuke Endoh]

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

Hello,

I believe I've found a bug in the IN6_IS_ADDR_V4COMPAT macro.
It incorrectly returns false for IPv4-compatible addresses
where the last octet is 1, such as ::192.168.1.1. It should return true.

## Reproduction Code

```
#include <stdio.h>
#include <arpa/inet.h>
#include <netinet/in.h>

int test(const char *str) {
    struct in6_addr addr;

    if (inet_pton(AF_INET6, str, &addr) != 1) {
        perror("inet_pton failed");
        return -1;
    }

    return IN6_IS_ADDR_V4COMPAT(&addr);
}

int main() {
    printf("IN6_IS_ADDR_V4COMPAT(::192.168.1.1) = %d\n",
test("::192.168.1.1"));
    printf("IN6_IS_ADDR_V4COMPAT(::192.168.1.2) = %d\n",
test("::192.168.1.2"));

    return 0;
}
```

## Expected Result (with glibc)

```
$ gcc -o test test.c && ./test
IN6_IS_ADDR_V4COMPAT(::192.168.1.1) = 1
IN6_IS_ADDR_V4COMPAT(::192.168.1.2) = 1
```

## Actual Result (with musl-gcc)

```
$ musl-gcc -o test test.c && ./test
IN6_IS_ADDR_V4COMPAT(::192.168.1.1) = 0
IN6_IS_ADDR_V4COMPAT(::192.168.1.2) = 1
```

As you can see, the macro returns false for ::192.168.1.1,
while it correctly returns true for ::192.168.1.2.

## Suggested Fix

I suspect the condition in the macro is incorrect.
Currently it checks if the LSB is "> 1", but it should be ">= 1".

Here is a proposed patch:

```
diff --git a/include/netinet/in.h b/include/netinet/in.h
index fb628b61..71bc26ab 100644
--- a/include/netinet/in.h
+++ b/include/netinet/in.h
@@ -132,7 +132,7 @@ uint16_t ntohs(uint16_t);

 #define IN6_IS_ADDR_V4COMPAT(a) \
         (((uint32_t *) (a))[0] == 0 && ((uint32_t *) (a))[1] == 0 && \
-         ((uint32_t *) (a))[2] == 0 && ((uint8_t *) (a))[15] > 1)
+         ((uint32_t *) (a))[2] == 0 && ((uint8_t *) (a))[15] >= 1)

 #define IN6_IS_ADDR_MC_NODELOCAL(a) \
         (IN6_IS_ADDR_MULTICAST(a) && ((((uint8_t *) (a))[1] & 0xf) == 0x1))
```

Thank you for your time and for maintaining this great project.

Best regards,
Yusuke

[-- Attachment #2: Type: text/html, Size: 2409 bytes --]

Re: Bug in IN6_IS_ADDR_V4COMPAT macro for addresses ending in .1

[Thorsten Glaser]

On Fri, 25 Jul 2025, Yusuke Endoh wrote:

>I suspect the condition in the macro is incorrect.
>Currently it checks if the LSB is "> 1", but it should be ">= 1".

The analysis is correct, the fix is wrong (::172.17.1.0 would still
be misrecognised).

AIUI, the macro needs to check that the address begins with quite a
lot of zeroes but is not exactly ::1 (or ::).

> #define IN6_IS_ADDR_V4COMPAT(a) \
>         (((uint32_t *) (a))[0] == 0 && ((uint32_t *) (a))[1] == 0 && \
>-         ((uint32_t *) (a))[2] == 0 && ((uint8_t *) (a))[15] > 1)
>+         ((uint32_t *) (a))[2] == 0 && ((uint8_t *) (a))[15] >= 1)

Is that casting well-defined? If so, perhaps:

 +         ((uint32_t *) (a))[2] == 0 && ((uint32_t *) (a))[3] > 1)

Untested, and I’ve not yet had enough coffee, so DWIM ☻

bye,
//mirabilos
-- 
(gnutls can also be used, but if you are compiling lynx for your own use,
there is no reason to consider using that package)
	-- Thomas E. Dickey on the Lynx mailing list, about OpenSSL

Re: Bug in IN6_IS_ADDR_V4COMPAT macro for addresses ending in .1

[Yusuke Endoh]

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

Hi Thorsten,

Thanks for the clarification, that makes sense.
I agree that your suggested approach is the correct one.

Let me know if there's anything I can do to help to move this forward.

For context, I am a developer for the programming language Ruby,
and I am experimenting with adding Alpine Linux to our CI matrix.
This bug is a blocker for me as it's causing multiple test failures.

Thanks,

Yusuke

[-- Attachment #2: Type: text/html, Size: 478 bytes --]

Re: Bug in IN6_IS_ADDR_V4COMPAT macro for addresses ending in .1

[Rich Felker]

On Fri, Jul 25, 2025 at 04:35:59PM +0200, Thorsten Glaser wrote:
> On Fri, 25 Jul 2025, Yusuke Endoh wrote:
> 
> >I suspect the condition in the macro is incorrect.
> >Currently it checks if the LSB is "> 1", but it should be ">= 1".
> 
> The analysis is correct, the fix is wrong (::172.17.1.0 would still
> be misrecognised).
> 
> AIUI, the macro needs to check that the address begins with quite a
> lot of zeroes but is not exactly ::1 (or ::).
> 
> > #define IN6_IS_ADDR_V4COMPAT(a) \
> >         (((uint32_t *) (a))[0] == 0 && ((uint32_t *) (a))[1] == 0 && \
> >-         ((uint32_t *) (a))[2] == 0 && ((uint8_t *) (a))[15] > 1)
> >+         ((uint32_t *) (a))[2] == 0 && ((uint8_t *) (a))[15] >= 1)
> 
> Is that casting well-defined?

Probably thanks to unions, but maybe not. In any case see the thread
by Brad House, [PATCH 1/1] IN6_IS_ADDR_LOOPBACK() and similar macros
warn on -Wcast-qual. It has a pending patch to do away with this mess
of dubious correctness that I was about to apply, but I see the bug
mentioned here is still present.

> If so, perhaps:
> 
>  +         ((uint32_t *) (a))[2] == 0 && ((uint32_t *) (a))[3] > 1)
> 
> Untested, and I’ve not yet had enough coffee, so DWIM ☻

This is not valid, You can't do order comparisons on the uint32_t
units because they're byte order dependent. But I suspect the actual
intent is not >1 but !=0.

Rich

Re: Bug in IN6_IS_ADDR_V4COMPAT macro for addresses ending in .1

[Thorsten Glaser]

On Fri, 25 Jul 2025, Rich Felker wrote:

>>  +         ((uint32_t *) (a))[2] == 0 && ((uint32_t *) (a))[3] > 1)
>>
>> Untested, and I’ve not yet had enough coffee, so DWIM ☻
>
>This is not valid, You can't do order comparisons on the uint32_t
>units because they're byte order dependent.

Oh, right.

>But I suspect the actual intent is not >1 but !=0.

No, it is exactly “NOT IN (0, 1)”.

Basically, :: (IPv6 any) and ::1 (IPv6 localhost) need to be excluded
from the sets of IPv6 addresses that begin with all-zero before an
embedded IPv4 address (except 0.0.0.0 and 0.0.0.1), and optionally
a :FFFF: before *that*, depending on use case (apparently not here).

+         ((uint32_t *) (a))[2] == 0 && ntohl((uint32_t *) (a))[3] > 1)

Maybe this?

bye,
//mirabilos
-- 
„Cool, /usr/share/doc/mksh/examples/uhr.gz ist ja ein Grund,
mksh auf jedem System zu installieren.“
	-- XTaran auf der OpenRheinRuhr, ganz begeistert
(EN: “[…]uhr.gz is a reason to install mksh on every system.”)

Re: Bug in IN6_IS_ADDR_V4COMPAT macro for addresses ending in .1

[Rich Felker]

On Sat, Jul 26, 2025 at 02:53:19AM +0200, Thorsten Glaser wrote:
> On Fri, 25 Jul 2025, Rich Felker wrote:
> 
> >>  +         ((uint32_t *) (a))[2] == 0 && ((uint32_t *) (a))[3] > 1)
> >>
> >> Untested, and I’ve not yet had enough coffee, so DWIM ☻
> >
> >This is not valid, You can't do order comparisons on the uint32_t
> >units because they're byte order dependent.
> 
> Oh, right.
> 
> >But I suspect the actual intent is not >1 but !=0.
> 
> No, it is exactly “NOT IN (0, 1)”.

Yeah, I realized this later. Oops.

> Basically, :: (IPv6 any) and ::1 (IPv6 localhost) need to be excluded
> from the sets of IPv6 addresses that begin with all-zero before an
> embedded IPv4 address (except 0.0.0.0 and 0.0.0.1), and optionally
> a :FFFF: before *that*, depending on use case (apparently not here).

V4MAPPED is a different thing than V4COMPAT. The latter is actually
useless and so probably nobody has cared about this macro being wrong
because it only comes up in tests, not in any actual usage.

> +         ((uint32_t *) (a))[2] == 0 && ntohl((uint32_t *) (a))[3] > 1)
> 
> Maybe this?

That's basically what glibc does. but we could also just do the simple
legible logic: top bits all zero && !IN6_IS_ADDR_UNSPECIFIED(a) &&
!IN6_IS_ADDR_LOOPBACK(a). All the redundant top-bits-zero checks
collapse out in CSE anyway.

Rich

Re: Bug in IN6_IS_ADDR_V4COMPAT macro for addresses ending in .1

[Rich Felker]

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

On Sat, Jul 26, 2025 at 05:47:58AM +0900, Yusuke Endoh wrote:
> Hi Thorsten,
> 
> Thanks for the clarification, that makes sense.
> I agree that your suggested approach is the correct one.
> 
> Let me know if there's anything I can do to help to move this forward.
> 
> For context, I am a developer for the programming language Ruby,
> and I am experimenting with adding Alpine Linux to our CI matrix.
> This bug is a blocker for me as it's causing multiple test failures.

Proposed fix.


[-- Attachment #2: 0001-fix-erroneous-definition-of-IN6_IS_ADDR_V4COMPAT.patch --]
[-- Type: text/plain, Size: 1781 bytes --]

From a6244de1c94588cd8cc965c15619d2649418f7a3 Mon Sep 17 00:00:00 2001
From: Rich Felker <dalias@aerifal.cx>
Date: Wed, 30 Jul 2025 09:32:02 -0400
Subject: [PATCH] fix erroneous definition of IN6_IS_ADDR_V4COMPAT

v4-compatible addresses in ipv6 are a deprecated feature where the
high 96 bits are all zero and an ipv4 address is stored in the low
32 bits. however, since :: and ::1 are the unspecified and loopback
addresses, these two particular values are excluded from the
definition of the v4-compat class.

our version of the macro incorrectly assessed this condition by
checking only the high 96 and low 8 bits. this incorrectly excluded
the v4compat version of any ipv4 address ending in .1, not just ::1.

rather than writing out non-obvious or error-prone conditions on the
individual address bytes, just express the "not :: or ::1" condition
naturally using the existing IN6_IS_ADDR_UNSPECIFIED and
IN6_IS_ADDR_LOOPBACK macros, after checking that the high 96 bits are
all zero. any vaguely reasonable compiler will collapse out the
redundant tests of the upper bits as part of CSE.
---
 include/netinet/in.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/netinet/in.h b/include/netinet/in.h
index fb628b61..60bbaa75 100644
--- a/include/netinet/in.h
+++ b/include/netinet/in.h
@@ -132,7 +132,8 @@ uint16_t ntohs(uint16_t);
 
 #define IN6_IS_ADDR_V4COMPAT(a) \
         (((uint32_t *) (a))[0] == 0 && ((uint32_t *) (a))[1] == 0 && \
-         ((uint32_t *) (a))[2] == 0 && ((uint8_t *) (a))[15] > 1)
+         ((uint32_t *) (a))[2] == 0 && \
+         !IN6_IS_ADDR_UNSPECIFIED(a) && !IN6_IS_ADDR_LOOPBACK(a))
 
 #define IN6_IS_ADDR_MC_NODELOCAL(a) \
         (IN6_IS_ADDR_MULTICAST(a) && ((((uint8_t *) (a))[1] & 0xf) == 0x1))
-- 
2.21.0

Re: [musl] Bug in IN6_IS_ADDR_V4COMPAT macro for addresses ending in .1

[Yusuke Endoh]

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

Hi Rich,

As a test, I applied your patch directly to the installed header file
in Alpine,
and confirmed Ruby's tests in question now pass.

Thank you very much!

2025年7月30日(水) 22:39 Rich Felker <dalias@libc.org>:

> On Sat, Jul 26, 2025 at 05:47:58AM +0900, Yusuke Endoh wrote:
> > Hi Thorsten,
> >
> > Thanks for the clarification, that makes sense.
> > I agree that your suggested approach is the correct one.
> >
> > Let me know if there's anything I can do to help to move this forward.
> >
> > For context, I am a developer for the programming language Ruby,
> > and I am experimenting with adding Alpine Linux to our CI matrix.
> > This bug is a blocker for me as it's causing multiple test failures.
>
> Proposed fix.
>
>

[-- Attachment #2: Type: text/html, Size: 1183 bytes --]