<!DOCTYPE html>
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p>I think value 0 is completely valid. It is important especially
when AI_PASSIVE is used. Then it allows auto-allocating new random
port. I do not see a reason why that should be refused as invalid.
I think that would cause regression.<br>
</p>
<p>At least AF_VSOCK (man 7 vsock) has svm_port defined as unsigned
int. I think that is the reason why AF_INET and AF_INET6 only were
restricted to 16bit value. I am not sure even bind() will work
with that. Socktype were checked because only TCP and UDP has
defined port to 16 bit unsigned int. I am not sure how
SOCK_SEQPACKET can be used over IP and whether it has the same
ports. I would assume yes, it would, but does someone actually
know that? Same with SOCK_DCCP.</p>
<p>It seems SOCK_RAW uses sockaddr_in, according to man page.
Therefore it should be included as well in ai_socktype check, if
that is still needed.<br>
</p>
<p>Is it safe to assume AF_INET and AF_INET6 has the same bit size
of port for any socket types? Is there any case known, where that
assumption based only on ai_family would not be true? I have never
used anything except inet protocols and unspec myself.</p>
<p>Regards,<br>
Petr<br>
</p>
<div class="moz-cite-prefix">On 02. 12. 24 16:54, Wilco Dijkstra
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:PAWPR08MB89823941E1D2523203FB77AB83352@PAWPR08MB8982.eurprd08.prod.outlook.com">
<pre class="moz-quote-pre" wrap="">Hi Petr,
So this looks like an alternative fix for this bug, previous proposal was [1] (and my review [2]).
@@ -2377,6 +2377,12 @@ getaddrinfo (const char *name, const char *service,
gaih_service.num = -1;
}
+ else if ((hints->ai_family == AF_INET || hints->ai_family == AF_INET6)
+ && (hints->ai_socktype == SOCK_STREAM || hints->ai_socktype == SOCK_DGRAM)
+ && gaih_service.num > UINT16_MAX)
+ {
+ return EAI_SERVICE;
+ }
The interesting thing is that this patch does something different than [1], both in
terms of overflow checking of strtoul() (which I think should be added) and the
return value when overflow is detected (where it's not clear what should be returned).</pre>
</blockquote>
I think EAI_SERVICE is better. <span style="white-space: pre-wrap">EAI_NONAME has somehow too many meaning already, too many ors already for its definition.</span>
<blockquote type="cite"
cite="mid:PAWPR08MB89823941E1D2523203FB77AB83352@PAWPR08MB8982.eurprd08.prod.outlook.com">
<pre class="moz-quote-pre" wrap="">
Also what is the reason for checking ai_family and ai_socktype too? Can the service
be outside the valid range 1..65535 in other cases, and thus we should support full
64-bit range?</pre>
</blockquote>
At least AF_VSOCK supports unsigned int. That is just 32 bits, but
more than common 16 bits.<br>
<blockquote type="cite"
cite="mid:PAWPR08MB89823941E1D2523203FB77AB83352@PAWPR08MB8982.eurprd08.prod.outlook.com">
<pre class="moz-quote-pre" wrap="">
Cheers,
Wilco
[1] <a class="moz-txt-link-freetext" href="https://sourceware.org/pipermail/libc-alpha/2024-September/159785.html">https://sourceware.org/pipermail/libc-alpha/2024-September/159785.html</a>
[2] <a class="moz-txt-link-freetext" href="https://sourceware.org/pipermail/libc-alpha/2024-November/161184.html">https://sourceware.org/pipermail/libc-alpha/2024-November/161184.html</a>
</pre>
</blockquote>
<pre class="moz-signature" cols="72">--
Petr Menšík
Senior Software Engineer, RHEL
Red Hat, <a class="moz-txt-link-freetext" href="http://www.redhat.com/">http://www.redhat.com/</a>
PGP: DFCF908DB7C87E8E529925BC4931CA5B6C9FC5CB</pre>
</body>
</html>