Subject: Re: racoon broken in -current: grab_myaddrs and SIOCGIFCONF
To: enami tsugutomo <enami@netbsd.org>
From: Greg Troxel <gdt@ir.bbn.com>
List: tech-net
Date: 08/30/2007 11:48:53
racoon doesn't work in current because SIOCGIFCONF is broken. (I know
racoon should probably not use SIOCGIFCONF, but instead getifaddrs. I
won't argue, but SIOCGIFCONF should work in any case.)
As far as I can tell there are two things wrong:
* commit to sys/net/if.h to include sockaddr_storage in struct ifreq.
This breaks ABI compatability and things don't seem versioned. I
don't understand the rationale for this change.
* sys/net/if.c:ifconf() uses struct sockaddr for size tests when the
dominant member of the union is sockaddr_storage. This used to be
correct (or rather a latent bug) before the change to sockaddr_storage
in if.h.
Right now, sockaddrs that fit in struct sockaddr cause a struct ifreq to
be the full size (144 bytes). But ones that don't (AF_LINK, AF_INET6)
are made to fit exactly, which makes them shorter. I have a kludge in
racoon tomatch this, and it then works.
I propose the following:
1. revert addition of sockaddr_storage. I would claim that using struct
sockaddr to place longer things is wrong; that's what sockaddr_storage
is for. But perhaps other uses of struct ifreq don't have this issue.
This restores ABI compat for SIOCGIFCONF, and makes step 2 protective
rather than critical.
2. fix the code in if.c:ifconf() to use sizeof(struct ifconf.ifr_ifru)
instead of knowing the internal structure, so that each reply will be
bigger than a struct ifreq only if the address doesn't fit (the original
API).
If step 1 is also done, step 2 will guard against unintentional changes
restore ABI compatibility. If 1 is not done, it will at least restore
API compatability.
Christos: can you explain which ioctls were trouble? I don't follow
from your commit message below.
Thanks,
Greg
src/sys/net/if.h:
revision 1.124
date: 2007/05/29 21:32:29; author: christos; state: Exp; lines: +2 -1
Add a sockaddr_storage member to "struct ifreq" maintaining backwards
compatibility with the older ioctls. This avoids stack smashing and
abuse of "struct sockaddr" when ioctls placed "struct sockaddr_foo's" that
were longer than "struct sockaddr".
XXX: Some of the emulations might be broken; I tried to add code for
them but I did not test them.
src/sys/net/if.c:
revision 1.190
date: 2007/06/01 09:35:47; author: enami; state: Exp; lines: +26 -32
Fix some bugs in ifconf():
- maintain space left correctly. the pointer is advanced by the size
of struct ifreq when length of address is small.
- single sizeof operator is enough to take the size of struct.
- the type of `sz' must be singed type since it is/was compared against to
the variable which may become negative.
- no need to traverse rest of interfaces once we got an error. note that
the latter `break' statement was inside inner loop.