NetBSD Problem Report #48964
From www@NetBSD.org Fri Jul 4 18:16:24 2014
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
by mollari.NetBSD.org (Postfix) with ESMTPS id 0CEA5A6547
for <gnats-bugs@gnats.NetBSD.org>; Fri, 4 Jul 2014 18:16:24 +0000 (UTC)
Message-Id: <20140704181622.D827AA654A@mollari.NetBSD.org>
Date: Fri, 4 Jul 2014 18:16:22 +0000 (UTC)
From: oshima-ya@yagoto-urayama.jp
Reply-To: oshima-ya@yagoto-urayama.jp
To: gnats-bugs@NetBSD.org
Subject: In urndis(4), initialization of ifnet structure is not enough, cause panic.
X-Send-Pr-Version: www-1.0
>Number: 48964
>Category: kern
>Synopsis: In urndis(4), initialization of ifnet structure is not enough, cause panic.
>Confidential: no
>Severity: critical
>Priority: high
>Responsible: skrll
>State: closed
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Fri Jul 04 18:20:00 +0000 2014
>Closed-Date: Sun Apr 05 07:10:50 +0000 2015
>Last-Modified: Sun Apr 05 07:10:50 +0000 2015
>Originator: Yasushi Oshima
>Release: NetBSD-current 6.99.45
>Organization:
>Environment:
NetBSD jaguar 6.99.45 NetBSD 6.99.45 (GENERIC) #3: Sat Jul 5 02:30:41 JST 2014 root@sweety:/export/current/daily/20140705/obj/amd64/sys/arch/amd64/compile/GENERIC amd64
>Description:
When running dhcpcd(4) with urndis(4), ioctl(SIOCINITIFADDR) is called and kernel panic occurs in ether_ioctl().
urndis_ioctl(SIOCINITIFADDR) will pass through to ether_ioctl(),
ether_ioctl(SIOCINITIFADDR) will call ifp->if_init() function.
Howerver, this is NULL, wasn't set when attaching urndis(4).
note:
This problem appears with dhcpcd version 6.4.0 (or later?)
Before 6.4 case, it will call ioctl(SOCSIFFLAGS) in the first. so, ifp->if_init() is not called by ether_ioctl().
This problem will exist in netbsd-6, but not appear.
>How-To-Repeat:
On netbsd-current kernel with fix PR kern/48963
and current's dhcpcd ver. 6.4.0 (after 2014.6.16).
1. Attach urndis(4) device
2. Run 'dhcpcd urndis0'
>Fix:
Set urndis_init() to ifp->if_init,
with change function type void to int (and also add return code)
--- if_urndis.c.old 2014-07-04 19:03:07.000000000 +0900
+++ if_urndis.c 2014-07-05 02:58:13.000000000 +0900
@@ -76,7 +76,7 @@
static int urndis_rx_list_init(struct urndis_softc *);
static int urndis_tx_list_init(struct urndis_softc *);
-static void urndis_init(struct ifnet *);
+static int urndis_init(struct ifnet *);
static void urndis_stop(struct ifnet *);
static usbd_status urndis_ctrl_msg(struct urndis_softc *, uint8_t, uint8_t,
@@ -1022,35 +1022,35 @@
}
#endif
-static void
+static int
urndis_init(struct ifnet *ifp)
{
struct urndis_softc *sc;
int i, s;
- usbd_status err;
+ usbd_status err=0;
sc = ifp->if_softc;
if (ifp->if_flags & IFF_RUNNING)
- return;
+ return err;
- if (urndis_ctrl_init(sc) != RNDIS_STATUS_SUCCESS)
- return;
+ if ((err=urndis_ctrl_init(sc)) != RNDIS_STATUS_SUCCESS)
+ return err;
s = splnet();
- if (urndis_tx_list_init(sc) == ENOBUFS) {
+ if ((err=urndis_tx_list_init(sc)) == ENOBUFS) {
printf("%s: tx list init failed\n",
DEVNAME(sc));
splx(s);
- return;
+ return err;
}
- if (urndis_rx_list_init(sc) == ENOBUFS) {
+ if ((err=urndis_rx_list_init(sc)) == ENOBUFS) {
printf("%s: rx list init failed\n",
DEVNAME(sc));
splx(s);
- return;
+ return err;
}
err = usbd_open_pipe(sc->sc_iface_data, sc->sc_bulkin_no,
@@ -1059,7 +1059,7 @@
printf("%s: open rx pipe failed: %s\n", DEVNAME(sc),
usbd_errstr(err));
splx(s);
- return;
+ return err;
}
err = usbd_open_pipe(sc->sc_iface_data, sc->sc_bulkout_no,
@@ -1068,7 +1068,7 @@
printf("%s: open tx pipe failed: %s\n", DEVNAME(sc),
usbd_errstr(err));
splx(s);
- return;
+ return err;
}
for (i = 0; i < RNDIS_RX_LIST_CNT; i++) {
@@ -1086,6 +1086,7 @@
ifp->if_flags &= ~IFF_OACTIVE;
splx(s);
+ return err;
}
static void
@@ -1440,6 +1441,7 @@
ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST;
ifp->if_start = urndis_start;
ifp->if_ioctl = urndis_ioctl;
+ ifp->if_init = urndis_init;
#if 0
ifp->if_watchdog = urndis_watchdog;
#endif
>Release-Note:
>Audit-Trail:
Responsible-Changed-From-To: kern-bug-people->skrll
Responsible-Changed-By: skrll@NetBSD.org
Responsible-Changed-When: Sat, 05 Jul 2014 09:22:50 +0000
Responsible-Changed-Why:
Take
From: "Nick Hudson" <skrll@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/48964 CVS commit: src/sys/dev/usb
Date: Sat, 5 Jul 2014 11:13:13 +0000
Module Name: src
Committed By: skrll
Date: Sat Jul 5 11:13:13 UTC 2014
Modified Files:
src/sys/dev/usb: if_urndis.c
Log Message:
PR/48964: In urndis(4), initialization of ifnet structure is not enough,
cause panic.
Update urndis_init to return and error and use as if_init based on the
patch in the PR with stylistic changes from me.
XXX IFF_RUNNING should be ignored here?
To generate a diff of this commit:
cvs rdiff -u -r1.7 -r1.8 src/sys/dev/usb/if_urndis.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
State-Changed-From-To: open->feedback
State-Changed-By: skrll@NetBSD.org
State-Changed-When: Sat, 05 Jul 2014 11:22:52 +0000
State-Changed-Why:
I committed a similar patch. Perhaps you can look at the IFF_RUNNING
check in urndis_init?
From: fukumoto@imasy.or.jp
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: kern/48964
Date: Sun, 06 Jul 2014 22:32:33 +0900 (JST)
When closing this pr, please close kern/46290 as well. Thank you.
FUKUMOTO Atsushi
fukumoto@imasy.or.jp
From: Yasushi Oshima <oshima-ya@yagoto-urayama.jp>
To: gnats-bugs@NetBSD.org, skrll@NetBSD.org, gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org
Cc:
Subject: Re: kern/48964 (In urndis(4), initialization of ifnet structure is
not enough, cause panic.)
Date: Mon, 07 Jul 2014 02:31:40 +0900 (JST)
> Perhaps you can look at the IFF_RUNNING
> check in urndis_init?
[from diff]:
if (ifp->if_flags & IFF_RUNNING)
- return;
+ return EBUSY;
I don't know whether if_init() will be called or not when IFF_RUNNING flag is ON,
it will depend on the design of network (or ethernet) driver.
It seems that many nic drivers are stopping I/O, resetting and re-initializing,
or some drivers skip lator and return with 0 in this case.
at least I could not find another drivers which returns with any ERROR.
Thanks.
--
Yasushi Oshima
From: "Nick Hudson" <skrll@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/48964 CVS commit: src/sys/dev/usb
Date: Sat, 19 Jul 2014 08:38:28 +0000
Module Name: src
Committed By: skrll
Date: Sat Jul 19 08:38:28 UTC 2014
Modified Files:
src/sys/dev/usb: if_urndis.c
Log Message:
Don't return an error in urndis_init if IFF_RUNNING is set on entry.
That is, fix my mistake in handling PR/48964
To generate a diff of this commit:
cvs rdiff -u -r1.8 -r1.9 src/sys/dev/usb/if_urndis.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: Yasushi Oshima <oshima-ya@yagoto-urayama.jp>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: PR/48964 CVS commit: src/sys/dev/usb
Date: Sat, 26 Jul 2014 00:49:51 +0900 (JST)
> Module Name: src
> Committed By: skrll
> Date: Sat Jul 19 08:38:28 UTC 2014
>
> Modified Files:
> src/sys/dev/usb: if_urndis.c
>
> Log Message:
> Don't return an error in urndis_init if IFF_RUNNING is set on entry.
>
> That is, fix my mistake in handling PR/48964
>
>
> To generate a diff of this commit:
> cvs rdiff -u -r1.8 -r1.9 src/sys/dev/usb/if_urndis.c
>
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
>
It works well in my environment.
please pull-up this to netbsd-6.
Thanks.
--
Yasushi Oshima
From: Yasushi Oshima <oshima-ya@yagoto-urayama.jp>
To: gnats-bugs@gnats.netbsd.org
Cc:
Subject: Re: kern/48964
Date: Sun, 05 Apr 2015 14:46:32 +0900 (JST)
> kern/48964 - critical high priority sw-bug
> In urndis(4), initialization of ifnet structure is not enough, cause panic.
> http://gnats.NetBSD.org/cgi-bin/query-pr-single.pl?number=48964
I found it has been already fixed in current, netbsd-6 include 6.1.5
and netbsd-7, so please close it.
I sent a feedback previously but this state is still feedback yet...
State-Changed-From-To: feedback->closed
State-Changed-By: martin@NetBSD.org
State-Changed-When: Sun, 05 Apr 2015 07:10:50 +0000
State-Changed-Why:
Confirmed fixed
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2007
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.