On 7/15/19 10:28 AM, Claudio Jeker wrote:
On Sun, Jul 14, 2019 at 01:15:40PM +0300, Vadim Penzin wrote:
Since `sa_len' describes the size of a `sockaddr' (or one of its
derivatives) /including/ `sa_len' (maybe I am wrong, but this is my
interpretation of the comment `total length' that appears near the
definition of `struct sockaddr' in <sys/socket.h>), `sa_len' just
cannot be zero.
Yes, it can not be zero.
The current definition of ROUNDUP() might hide a bug. In addition, on
some machines, it disturbs the pipeline of the CPU by introducing a
branch (for no real reason, as it seems, while I might be
nitpicking). At very least, it looks confusing.
The following patch amends the issue:
Index: route.c
===================================================================
RCS file: /cvs/src/sys/net/route.c,v
retrieving revision 1.387
diff -u -p -r1.387 route.c
--- route.c 24 Jun 2019 22:26:25 -0000 1.387
+++ route.c 14 Jul 2019 10:05:04 -0000
@@ -138,7 +138,7 @@
#include <net/bfd.h>
#endif
-#define ROUNDUP(a) (a>0 ? (1 + (((a) - 1) | (sizeof(long) - 1))) :
sizeof(long))
+#define ROUNDUP(a) (1 + (((a) - 1) | (sizeof(long) - 1)))
/* Give some jitter to hash, to avoid synchronization between routers. */
static uint32_t rt_hashjitter;
This only touches only one version of ROUNDUP() and looking at how ROUNDUP()
is used in route.c I wonder if it is actually needed at all.
Looking at the code in route.c and rtsock.c I think it is better to leave
this like it is or fix both files making sure that the necessary checks
are put in place to make sure that a sa_len of 0 can't get into the system.
If you wish to remove ROUNDUP() then fine, however, leaving it as is
does not help.
Imagine the behaviour of the original ROUNDUP() in the unlikely event
when the kernel does somehow produce a sockaddr with sa_len of zero.
The kernel will send an unvalid sockaddr to the user. ROUNDUP() alone
will not help detection or correction of that fault.
Now imagine the behaviour of the new version. The kernel will not send
an incorrect sockaddr to the user (ROUNDUP() returns zero) but a
sockaddr will be missing entirely.
What is better: having something unusable and misleading or not having
it (with a good way of knowing that it is missing)? That's philosophy
to me.
In any case, ROUNDUP() cannot magically help when sa_len is zero.
