Subject: Re: racoon broken in -current: grab_myaddrs and SIOCGIFCONF
To: Greg Troxel , enami tsugutomo <enami@netbsd.org>
From: Christos Zoulas <christos@zoulas.com>
List: tech-net
Date: 08/31/2007 08:53:47
On Aug 30, 11:48am, gdt@ir.bbn.com (Greg Troxel) wrote:
-- Subject: Re: racoon broken in -current: grab_myaddrs and SIOCGIFCONF

| 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.

The versioning happens automagically because the ioctl number changes
since sizeof(struct ifreq) changes.

| * 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.

I am not sure about that. I will have to look, but now I have no source
access and I am on dialup :-(

| 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.

Ones where sizeof(ioctl struct used) == sizeof(struct oifreq). But that
has been fixed since.

christos