NetBSD Problem Report #35897

From kre@munnari.OZ.AU  Fri Mar  2 09:00:39 2007
Return-Path: <kre@munnari.OZ.AU>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by narn.NetBSD.org (Postfix) with ESMTP id 60AE563B896
	for <gnats-bugs@gnats.NetBSD.org>; Fri,  2 Mar 2007 09:00:39 +0000 (UTC)
Message-Id: <200703020900.l2290UPt012855@jade.coe.psu.ac.th>
Date: Fri, 2 Mar 2007 16:00:30 +0700 (ICT)
From: kre@munnari.OZ.AU
To: gnats-bugs@NetBSD.org
Subject: SIOCSIFALIFETIME_IN6 has a totally botched (unusable) interface
X-Send-Pr-Version: 3.95

>Number:         35897
>Category:       kern
>Synopsis:       SIOCSIFALIFETIME_IN6 has a totally botched (unusable) interface
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Mar 02 09:05:00 +0000 2007
>Closed-Date:    Sun Nov 18 09:41:57 +0000 2007
>Last-Modified:  Sun Nov 18 11:25:01 +0000 2007
>Originator:     Robert Elz
>Release:        NetBSD 3.99.15  (verified still in current of today)
>Organization:
	Prince of Songkla University
>Environment:
System: NetBSD jade.coe.psu.ac.th 3.99.15 NetBSD 3.99.15 (GENERIC-1.696-20060125) #8: Wed Jan 25 04:59:39 ICT 2006 kre@jade.coe.psu.ac.th:/usr/obj/current/kernels/JADE_ASUS i386
Architecture: i386
Machine: i386
>Description:
	The KAME code provided an ioctl to set the lifetimes of
	an IPv6 address - that is, SIOCSIFALIFETIME_IN6 (it also
	provides one to fetch those lifetimes, which does not quite
	have the problem described below, but is very ugly because of
	the same issue).

	The arg data to SIOCSIFALIFETIME_IN6 is the fairly generic
	struct in6_ifreq that handles lots of IPv6 ioctl data.

	The problem is, that this general interface provides just the
	interface name, and the ioctl specific data (which in this case
	is the ifru_lifetime field in the ifr_ifru union.   ifru_lifetime
	contains everything necessary to set the lifetimes for an
	address - except which address they're to be set for...

	As a specific address is needed, the v6 address ioctl
	code simply uses the ifru_addr of the in6_ifreq to select
	the address to which the ioctl is to apply.

	Unfortunately, ifr_ifru is a union, ifru_addr and ifru_lifetime
	occupy the same bits in the request.   Unless the lifetimes
	that need to be set just happen to bear a startling resemblance
	to the address to which they are to be applied, this cannot
	possibly work.

	Someone should audit all the SIOCS... IPv6 ioctls to see if
	any others have the same problem (there might be more than this one).

>How-To-Repeat:
	UTSL.

	The relevent source is in netinet6/in6.c - search for
	SIOCSIFALIFETIME_IN6

	The first instance amounts to ...

		switch (cmd) {
		/* ... */
		case SIOCSIFALIFETIME_IN6:
			sa6 = &ifr->ifr_addr;
			break;
		/* ... */
		}

		if (sa6 && sa6->sin6_family == AF_INET6) {
			/* ... */
			ia = in6ifa_ifpwithaddr(ifp, &sa6->sin6_addr);
		}

	/* ia is now the address upon which the ioctl is to operate */

	Note that ifr_addr is (in net/if.h) defined to be ifr_ifru.ifru_addr

	The next occurrence of SIOCSIFALIFETIME_IN6 does some
	sanity checking ...

		switch (cmd) {
		/* ...*/
		case SIOCSIFALIFETIME_IN6:
		    {
			struct in6_addrlifetime *lt;

			if (!privileged)
				return EPERM;
			if (ia == NULL)
				return EADDRNOTAVAIL;
			/* sanity for overflow - beware unsigned */
			lt = &ifr->ifr_ifru.ifru_lifetime;

			/* ... (actual checks look OK) ... */
		    }
		/* ... */
		}

	And then the third, and final, reference actually does the work...

		switch (cmd) {
		/* ... */
		case SIOCSIFALIFETIME_IN6:
			ia->ia6_lifetime = ifr->ifr_ifru.ifru_lifetime;
			/* ... */
			break;
		/* ... */
		}

	One union with two fields (ifru_addr and ifru_lifetime) cannot contain
	values for both at the same time.   Yet, that is what this interface
	assumes happens (and relies upon).

>Fix:
	Delete SIOCSIFALIFETIME_IN6 (obliterate it from all existence).

	Perhaps recognise it in the kernel and unconditionally return
	an error (EINVAL or something) as is done for a bunch of other
	obsolete (experimental) v6 ioctls that never survived into real use.
	(That's just to document what it used to be, as an unknown ioctl
	will also result in EINVAL).

	It cannot be safely used, and (fortunately) in the NetBSD tree
	at least, isn't used (anywhere I could find - only in in6.c
	(above) and where it is defined (in in6_var.h)).

	Since the function it provided isn't completely useless, it
	might be worth providing using a different interface (and
	consequently, perhaps unfortunately, a different ioctl name).

	On the other hand, as nothing currently uses that interface
	(address lifetimnes are set along with their addresses using
	the SIOCAIFADDR_IN6 request) then perhaps the interface isn't
	really needed, and we can get by without a replacement.

	Because it cannot work, and can never have worked, there is
	no need to provide any kind of compatibility support for this
	when it is removed.

	SIOCGIFALIFETIME_IN6 works OK, as the address part of the
	union is used to locate the address upon which to operate
	before the lifetime values overwrite the address value in the
	data struct passed to the ioctl.  What's more, this one
	actually is used (by ifconfig) so needs to be retained for
	backwards binary compatability, whatever else is done with it.

	If a new interface is designed to set the address
	lifetimes (aside from SIOCAIFADDR_IN6 - whch sets much more)
	then an analog (and cleaner) interface to fetch the lifetimes
	should be provided as well, and SIOCGIFALIFETIME_IN6 reduced
	to a COMPAT_XX type interface.

	From what I can see, the FreeBSD code is identical to that
	in NetBSD, and has exactly the same problem.   DragonFly
	seems to have slightly different code, but the same problem
	exists there.   The last KAME code still contains this problem.
	(OpenBSD I don't have immediate access too - that is, I don't
	have my own copy of their repository).   Most likely everyone
	with the KAME code (and anyone with the same interface) is
	going to suffer from the same problem.

	This problem will have existed in NetBSD (though in different
	forms) since the IPv6 code was first committed - a variant of
	this existed in in6.c 1.2 (the actual details differ, but there
	is an aliased memory problem that prevents this ioctl from
	working).   It is clear that it was never tested...

	The fix (deleting SIOCSIFALIFETIME_IN6) should be pulled up to
	all currently maintained versions of NetBSD.   It really should
	be gone before NetBSD 4 is released.

>Release-Note:

>Audit-Trail:
I just commmited a change to remove the ioctl from -current.

State-Changed-From-To: open->feedback
State-Changed-By: gdt@netbsd.org
State-Changed-When: Tue, 11 Sep 2007 19:55:32 +0000
State-Changed-Why:
fix commited to current


From: Robert Elz <kre@munnari.OZ.AU>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/35897 (SIOCSIFALIFETIME_IN6 has a totally botched (unusable) interface) 
Date: Sat, 17 Nov 2007 23:26:11 +0700

     Date:        Tue, 11 Sep 2007 19:55:34 +0000 (UTC)
     From:        gdt@NetBSD.org
     Message-ID:  <20070911195534.B557A63B86C@narn.NetBSD.org>

   | Synopsis: SIOCSIFALIFETIME_IN6 has a totally botched (unusable) interface
   | 
   | State-Changed-From-To: open->feedback
   | fix commited to current

 Sorry, I didn't notice this went into feedback state ...   I'm not sure
 what feedback I can possibly give about this one.   It's gone, good
 riddance!

 I would have liked to see this fixed in 4.0 as well, but I guess it is
 too late for that now.   It should also go away from the 3.x series.

 Please close this PR!

 Thanks,

 kre

State-Changed-From-To: feedback->closed
State-Changed-By: wiz@netbsd.org
State-Changed-When: Sun, 18 Nov 2007 09:41:57 +0000
State-Changed-Why:
Submitter asks to close the PR.


From: Robert Elz <kre@munnari.OZ.AU>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/35897 (SIOCSIFALIFETIME_IN6 has a totally botched (unusable) interface) 
Date: Sun, 18 Nov 2007 18:21:26 +0700

     Date:        Sun, 18 Nov 2007 09:41:58 +0000 (UTC)
     From:        wiz@NetBSD.org
     Message-ID:  <20071118094158.B757963B88C@narn.NetBSD.org>

   | Submitter asks to close the PR.

 A better log message might have been "PR has been resolved" - it isn't
 that the issue in the PR just "went away" or was a mistake, it was fixed...

 kre

>Unformatted:

Home	PR Database Search