NetBSD Problem Report #36390

From tls@panix.com  Sun May 27 17:48:57 2007
Return-Path: <tls@panix.com>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by narn.NetBSD.org (Postfix) with ESMTP id 22A7663B880
	for <gnats-bugs@gnats.NetBSD.org>; Sun, 27 May 2007 17:48:57 +0000 (UTC)
Message-Id: <200705271748.l4RHmt905659@panix5.panix.com>
Date: Sun, 27 May 2007 13:48:55 -0400 (EDT)
From: tls@netbsd.org
To: gnats-bugs@NetBSD.org
Subject: sockaddr_dl doesn't fit in sockaddr, thus ifreq ioctls broken
X-Send-Pr-Version: 3.95

>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
>Last-Modified:  Sun May 27 18:15:02 +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.

>Audit-Trail:
From: Thor Lancelot Simon <tls@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: PR/36390 CVS commit: src/usr.sbin/pppd
Date: Sun, 27 May 2007 18:11:38 +0000 (UTC)

 Module Name:	src
 Committed By:	tls
 Date:		Sun May 27 18:11:38 UTC 2007

 Modified Files:
 	src/usr.sbin/pppd: Makefile.inc
 	src/usr.sbin/pppd/pppd: sys-bsd.c

 Log Message:
 Gross workaround for PR 36390: don't overwrite the stack with zeroes when
 using struct sockaddr_dl in an ifreq.


 To generate a diff of this commit:
 cvs rdiff -r1.3 -r1.4 src/usr.sbin/pppd/Makefile.inc
 cvs rdiff -r1.55 -r1.56 src/usr.sbin/pppd/pppd/sys-bsd.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

Home	PR Database Search