>Number:         36390
>Category:       kern
>Synopsis:       struct ifreq has sockaddr, but ifreq ioctls use sockaddr_dl
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun May 27 17:50:00 +0000 2007
>Originator:     Thor Lancelot Simon
>Release:        NetBSD 4.99.19
>Organization:
The NetBSD Foundation
>Environment:
NetBSD claw 4.99.19 NetBSD 4.99.19 (GENERIC.MP) #0: Sun May 13 21:52:49 PDT
2007
builds@wb25:/home/builds/ab/HEAD/i386/200705130002Z-obj/home/builds/ab/HEAD/src/sys/arch/i386/compile/GENERIC.MP
i386
Architecture: i386
Machine: i386
>Description:
 Various interface ioctls take struct ifreq, which contains a union
whose largest member is struct sockaddr.  But struct sockaddr is
really variable-size and should thus never be embedded in other
structures -- sockaddr_storage is guaranteed to be as large as the
largest sockaddr_xxx type and can thus be used for this purpose if
absolutely necessary.

Unfortunately, some of these ioctls return data-link addresses (sockaddr_dl)
and the maximum size of such an address exceeds the size of struct sockaddr
by 4 bytes.  Efforts at good programming practice (e.g. zeroing the
sockaddr_dl before use as at pppd/pppd/bsd-sys.c around line 1810) can
thus smash the stack and cause problems; worse, if we actually had any
devices with link-layer addresses larger than 12 bytes they would always
be truncated when returned to userspace by ioctl (unless the kernel blithely
ran off the end of the ifreq structure instead, even worse).

inet6 and some other protocols have their own ifreq structure and ioctl
specifically to work around this problem but there is none such for data-link
addresses.

>How-To-Repeat:

You can see what's going on pretty quickly if you try to build pppd with
-D_FORTIFY_SOURCE, which checks the length of the arguments to memset,
memcpy, etc.

>Fix:

Fixing this, one way or the other, will require versioning all the ioctls
that take an ifreq.  Either struct ifreq has to grow (by using struct
sockaddr_storage in the union instead of sockaddr) or, ideally, the interface
ioctls should be converted to proplib.