Message ID | 20221104131031.850850-2-s.hauer@pengutronix.de |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:6687:0:0:0:0:0 with SMTP id l7csp392065wru; Fri, 4 Nov 2022 06:11:37 -0700 (PDT) X-Google-Smtp-Source: AMsMyM682B0iv6rtnKqYEy3oyw+18Mk5x+WeU5sC53VuTaY+J+Igcz29+rPvCdp9/e1Jh3oPPSjd X-Received: by 2002:a17:903:41cc:b0:186:b756:a5f0 with SMTP id u12-20020a17090341cc00b00186b756a5f0mr35577170ple.132.1667567496937; Fri, 04 Nov 2022 06:11:36 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1667567496; cv=none; d=google.com; s=arc-20160816; b=mOsaX4jafo0JiSGY2vHuHy+lEidACc5Ej+87fm3xGj+DisBlfaLkiKzwMsKgYKSdui fS5ktRR2//MLq4rLEDbl7/fkEdEsbQJUXGZ86CW3RuRP6d928Ynje4iaHpz8HKq3fpyU maI/foERAYOEmVIOFx2OOJDmSD+e4N/G28kEuZEMqjOhQ94raiVPmqI9ULBgd5/co2LN Ev5WTrlb9dMDvBIpO3W/h/nmXQCGZpfMjGLeVNp+YGxtrc0//tPQPpSojlN1oOvf5V0h UYQsTOMjcvSQtdcuU8pE2eGKgXGO227x+pTLrZK5xmu/nylccYLl39OpWbt4n2app3m8 0k3A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from; bh=Wcca4ILqaO7Rr4N4Hr+TmOHJCHvqcbbnhwCBC0vAZC0=; b=yR+DBy3hZxvqYvcx8diJoOm8uw7nrYMUZbjyD7/nY3r+g6vWxhPT23FT6VdTo5couN XCjVSeLlyDYgEPpr8DAY6s5oiWQwhtaSZ8UI2XTYWGG3o+i1VFXV4PNJhV1Fx0WqiVWs eI4N8kyf/kx4l6/K96MdEyLqns8CJnEbnyysvkPcFtX+cyKJhnvumwrPNrXlEyq2QQDB 9fuip7QUI64ZRrT8SYU7DVn77kSoA2Vdv3Cg8Gj3dttamcEWqUojIhVEh/A1n6JEhI8m q2WJ+iooZLy4HNzoK5tHg3wzMBeqItbBMxRLCY0SjWNb5KXzieuA4Ksv93e347dm/TmC Jz1w== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id l15-20020a170903244f00b0018388edd187si5518734pls.56.2022.11.04.06.11.22; Fri, 04 Nov 2022 06:11:36 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231747AbiKDNKn (ORCPT <rfc822;jimliu8233@gmail.com> + 99 others); Fri, 4 Nov 2022 09:10:43 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45770 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231634AbiKDNKi (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 4 Nov 2022 09:10:38 -0400 Received: from metis.ext.pengutronix.de (metis.ext.pengutronix.de [IPv6:2001:67c:670:201:290:27ff:fe1d:cc33]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0B2A42CDDA for <linux-kernel@vger.kernel.org>; Fri, 4 Nov 2022 06:10:37 -0700 (PDT) Received: from drehscheibe.grey.stw.pengutronix.de ([2a0a:edc0:0:c01:1d::a2]) by metis.ext.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from <sha@pengutronix.de>) id 1oqwSw-0000Yo-Sl; Fri, 04 Nov 2022 14:10:34 +0100 Received: from [2a0a:edc0:0:1101:1d::28] (helo=dude02.red.stw.pengutronix.de) by drehscheibe.grey.stw.pengutronix.de with esmtp (Exim 4.94.2) (envelope-from <sha@pengutronix.de>) id 1oqwSx-002HHl-3T; Fri, 04 Nov 2022 14:10:34 +0100 Received: from sha by dude02.red.stw.pengutronix.de with local (Exim 4.94.2) (envelope-from <sha@pengutronix.de>) id 1oqwSv-004043-44; Fri, 04 Nov 2022 14:10:33 +0100 From: Sascha Hauer <s.hauer@pengutronix.de> To: linux-usb@vger.kernel.org Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>, linux-kernel@vger.kernel.org, kernel@pengutronix.de, Sascha Hauer <s.hauer@pengutronix.de> Subject: [PATCH 1/2] usb: gadget: u_ether: Do not make UDC parent of the net device Date: Fri, 4 Nov 2022 14:10:30 +0100 Message-Id: <20221104131031.850850-2-s.hauer@pengutronix.de> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20221104131031.850850-1-s.hauer@pengutronix.de> References: <20221104131031.850850-1-s.hauer@pengutronix.de> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SA-Exim-Connect-IP: 2a0a:edc0:0:c01:1d::a2 X-SA-Exim-Mail-From: sha@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-kernel@vger.kernel.org X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_MED, SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1748571255867682636?= X-GMAIL-MSGID: =?utf-8?q?1748571255867682636?= |
Series |
use-after-free issues in configfs
|
|
Commit Message
Sascha Hauer
Nov. 4, 2022, 1:10 p.m. UTC
The UDC is not a suitable parent of the net device as the UDC can
change or vanish during the lifecycle of the ethernet gadget. This
can be illustrated with the following:
mkdir -p /sys/kernel/config/usb_gadget/mygadget
cd /sys/kernel/config/usb_gadget/mygadget
mkdir -p configs/c.1/strings/0x409
echo "C1:Composite Device" > configs/c.1/strings/0x409/configuration
mkdir -p functions/ecm.usb0
ln -s functions/ecm.usb0 configs/c.1/
echo "dummy_udc.0" > UDC
rmmod dummy_hcd
The 'rmmod' removes the UDC from the just created gadget, leaving
the still existing net device with a no longer existing parent.
Accessing the ethernet device with commands like:
ip --details link show usb0
will result in a KASAN splat:
==================================================================
BUG: KASAN: use-after-free in if_nlmsg_size+0x3e8/0x528
Read of size 4 at addr c5c84754 by task ip/357
CPU: 3 PID: 357 Comm: ip Not tainted 6.1.0-rc3-00013-gd14953726b24-dirty #324
Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
unwind_backtrace from show_stack+0x10/0x14
show_stack from dump_stack_lvl+0x58/0x70
dump_stack_lvl from print_report+0x134/0x4d4
print_report from kasan_report+0x78/0x10c
kasan_report from if_nlmsg_size+0x3e8/0x528
if_nlmsg_size from rtnl_getlink+0x2b4/0x4d0
rtnl_getlink from rtnetlink_rcv_msg+0x1f4/0x674
rtnetlink_rcv_msg from netlink_rcv_skb+0xb4/0x1f8
netlink_rcv_skb from netlink_unicast+0x294/0x478
netlink_unicast from netlink_sendmsg+0x328/0x640
netlink_sendmsg from ____sys_sendmsg+0x2a4/0x3b4
____sys_sendmsg from ___sys_sendmsg+0xc8/0x12c
___sys_sendmsg from sys_sendmsg+0xa0/0x120
sys_sendmsg from ret_fast_syscall+0x0/0x1c
Solve this by not setting the parent of the ethernet device.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/usb/gadget/function/u_ether.c | 4 ----
1 file changed, 4 deletions(-)
Comments
Hi Sascha, Greg, I have a breakage in 6.2-rc* that I eventually bisected to this commit, on a Ingenic SoC (using the jz4740 musb driver) with ECM or RNDIS configured through gadgetfs. When plugging the board to my PC, the USB network interface is recognized, but 'ip link' sees it as 'NO-CARRIER'. With this commit reverted on v6.2-rc5, everything works fine. Cheers, -Paul Le vendredi 04 novembre 2022 à 14:10 +0100, Sascha Hauer a écrit : > The UDC is not a suitable parent of the net device as the UDC can > change or vanish during the lifecycle of the ethernet gadget. This > can be illustrated with the following: > > mkdir -p /sys/kernel/config/usb_gadget/mygadget > cd /sys/kernel/config/usb_gadget/mygadget > mkdir -p configs/c.1/strings/0x409 > echo "C1:Composite Device" > configs/c.1/strings/0x409/configuration > mkdir -p functions/ecm.usb0 > ln -s functions/ecm.usb0 configs/c.1/ > echo "dummy_udc.0" > UDC > rmmod dummy_hcd > > The 'rmmod' removes the UDC from the just created gadget, leaving > the still existing net device with a no longer existing parent. > > Accessing the ethernet device with commands like: > > ip --details link show usb0 > > will result in a KASAN splat: > > ================================================================== > BUG: KASAN: use-after-free in if_nlmsg_size+0x3e8/0x528 > Read of size 4 at addr c5c84754 by task ip/357 > > CPU: 3 PID: 357 Comm: ip Not tainted 6.1.0-rc3-00013-gd14953726b24- > dirty #324 > Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree) > unwind_backtrace from show_stack+0x10/0x14 > show_stack from dump_stack_lvl+0x58/0x70 > dump_stack_lvl from print_report+0x134/0x4d4 > print_report from kasan_report+0x78/0x10c > kasan_report from if_nlmsg_size+0x3e8/0x528 > if_nlmsg_size from rtnl_getlink+0x2b4/0x4d0 > rtnl_getlink from rtnetlink_rcv_msg+0x1f4/0x674 > rtnetlink_rcv_msg from netlink_rcv_skb+0xb4/0x1f8 > netlink_rcv_skb from netlink_unicast+0x294/0x478 > netlink_unicast from netlink_sendmsg+0x328/0x640 > netlink_sendmsg from ____sys_sendmsg+0x2a4/0x3b4 > ____sys_sendmsg from ___sys_sendmsg+0xc8/0x12c > ___sys_sendmsg from sys_sendmsg+0xa0/0x120 > sys_sendmsg from ret_fast_syscall+0x0/0x1c > > Solve this by not setting the parent of the ethernet device. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > drivers/usb/gadget/function/u_ether.c | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/drivers/usb/gadget/function/u_ether.c > b/drivers/usb/gadget/function/u_ether.c > index e06022873df16..8f12f3f8f6eeb 100644 > --- a/drivers/usb/gadget/function/u_ether.c > +++ b/drivers/usb/gadget/function/u_ether.c > @@ -798,7 +798,6 @@ struct eth_dev *gether_setup_name(struct > usb_gadget *g, > net->max_mtu = GETHER_MAX_MTU_SIZE; > > dev->gadget = g; > - SET_NETDEV_DEV(net, &g->dev); > SET_NETDEV_DEVTYPE(net, &gadget_type); > > status = register_netdev(net); > @@ -873,8 +872,6 @@ int gether_register_netdev(struct net_device > *net) > struct usb_gadget *g; > int status; > > - if (!net->dev.parent) > - return -EINVAL; > dev = netdev_priv(net); > g = dev->gadget; > > @@ -905,7 +902,6 @@ void gether_set_gadget(struct net_device *net, > struct usb_gadget *g) > > dev = netdev_priv(net); > dev->gadget = g; > - SET_NETDEV_DEV(net, &g->dev); > } > EXPORT_SYMBOL_GPL(gether_set_gadget); >
Linux regression tracking (Thorsten Leemhuis)
Feb. 3, 2023, 12:46 p.m. UTC |
#2
Addressed
Unaddressed
[TLDR: I'm adding this report to the list of tracked Linux kernel regressions; the text you find below is based on a few templates paragraphs you might have encountered already in similar form. See link in footer if these mails annoy you.] On 01.02.23 14:32, Paul Cercueil wrote: > Hi Sascha, Greg, > > I have a breakage in 6.2-rc* that I eventually bisected to this commit, > on a Ingenic SoC (using the jz4740 musb driver) with ECM or RNDIS > configured through gadgetfs. > > When plugging the board to my PC, the USB network interface is > recognized, but 'ip link' sees it as 'NO-CARRIER'. With this commit > reverted on v6.2-rc5, everything works fine. [CCing the regression list, as it should be in the loop for regressions: https://docs.kernel.org/admin-guide/reporting-regressions.html] Thanks for the report. To be sure the issue doesn't fall through the cracks unnoticed, I'm adding it to regzbot, the Linux kernel regression tracking bot: #regzbot ^introduced 321b59870f850 #regzbot title usb: gadget: USB network interface is recognized, but 'ip link' sees it as 'NO-CARRIER' #regzbot ignore-activity This isn't a regression? This issue or a fix for it are already discussed somewhere else? It was fixed already? You want to clarify when the regression started to happen? Or point out I got the title or something else totally wrong? Then just reply and tell me -- ideally while also telling regzbot about it, as explained by the page listed in the footer of this mail. Developers: When fixing the issue, remember to add 'Link:' tags pointing to the report (the parent of this mail). See page linked in footer for details. Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr That page also explains what to do if mails like this annoy you. > Le vendredi 04 novembre 2022 à 14:10 +0100, Sascha Hauer a écrit : >> The UDC is not a suitable parent of the net device as the UDC can >> change or vanish during the lifecycle of the ethernet gadget. This >> can be illustrated with the following: >> >> mkdir -p /sys/kernel/config/usb_gadget/mygadget >> cd /sys/kernel/config/usb_gadget/mygadget >> mkdir -p configs/c.1/strings/0x409 >> echo "C1:Composite Device" > configs/c.1/strings/0x409/configuration >> mkdir -p functions/ecm.usb0 >> ln -s functions/ecm.usb0 configs/c.1/ >> echo "dummy_udc.0" > UDC >> rmmod dummy_hcd >> >> The 'rmmod' removes the UDC from the just created gadget, leaving >> the still existing net device with a no longer existing parent. >> >> Accessing the ethernet device with commands like: >> >> ip --details link show usb0 >> >> will result in a KASAN splat: >> >> ================================================================== >> BUG: KASAN: use-after-free in if_nlmsg_size+0x3e8/0x528 >> Read of size 4 at addr c5c84754 by task ip/357 >> >> CPU: 3 PID: 357 Comm: ip Not tainted 6.1.0-rc3-00013-gd14953726b24- >> dirty #324 >> Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree) >> unwind_backtrace from show_stack+0x10/0x14 >> show_stack from dump_stack_lvl+0x58/0x70 >> dump_stack_lvl from print_report+0x134/0x4d4 >> print_report from kasan_report+0x78/0x10c >> kasan_report from if_nlmsg_size+0x3e8/0x528 >> if_nlmsg_size from rtnl_getlink+0x2b4/0x4d0 >> rtnl_getlink from rtnetlink_rcv_msg+0x1f4/0x674 >> rtnetlink_rcv_msg from netlink_rcv_skb+0xb4/0x1f8 >> netlink_rcv_skb from netlink_unicast+0x294/0x478 >> netlink_unicast from netlink_sendmsg+0x328/0x640 >> netlink_sendmsg from ____sys_sendmsg+0x2a4/0x3b4 >> ____sys_sendmsg from ___sys_sendmsg+0xc8/0x12c >> ___sys_sendmsg from sys_sendmsg+0xa0/0x120 >> sys_sendmsg from ret_fast_syscall+0x0/0x1c >> >> Solve this by not setting the parent of the ethernet device. >> >> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> >> --- >> drivers/usb/gadget/function/u_ether.c | 4 ---- >> 1 file changed, 4 deletions(-) >> >> diff --git a/drivers/usb/gadget/function/u_ether.c >> b/drivers/usb/gadget/function/u_ether.c >> index e06022873df16..8f12f3f8f6eeb 100644 >> --- a/drivers/usb/gadget/function/u_ether.c >> +++ b/drivers/usb/gadget/function/u_ether.c >> @@ -798,7 +798,6 @@ struct eth_dev *gether_setup_name(struct >> usb_gadget *g, >> net->max_mtu = GETHER_MAX_MTU_SIZE; >> >> dev->gadget = g; >> - SET_NETDEV_DEV(net, &g->dev); >> SET_NETDEV_DEVTYPE(net, &gadget_type); >> >> status = register_netdev(net); >> @@ -873,8 +872,6 @@ int gether_register_netdev(struct net_device >> *net) >> struct usb_gadget *g; >> int status; >> >> - if (!net->dev.parent) >> - return -EINVAL; >> dev = netdev_priv(net); >> g = dev->gadget; >> >> @@ -905,7 +902,6 @@ void gether_set_gadget(struct net_device *net, >> struct usb_gadget *g) >> >> dev = netdev_priv(net); >> dev->gadget = g; >> - SET_NETDEV_DEV(net, &g->dev); >> } >> EXPORT_SYMBOL_GPL(gether_set_gadget); >> >
On Wed, Feb 01, 2023 at 01:32:51PM +0000, Paul Cercueil wrote: > Hi Sascha, Greg, > > I have a breakage in 6.2-rc* that I eventually bisected to this commit, > on a Ingenic SoC (using the jz4740 musb driver) with ECM or RNDIS > configured through gadgetfs. > > When plugging the board to my PC, the USB network interface is > recognized, but 'ip link' sees it as 'NO-CARRIER'. With this commit > reverted on v6.2-rc5, everything works fine. Ick, that's not good. Can you send a revert for this? Sascha, any ideas? thanks, greg k-h
Hi Greg, Le mercredi 08 février 2023 à 13:06 +0100, Greg Kroah-Hartman a écrit : > On Wed, Feb 01, 2023 at 01:32:51PM +0000, Paul Cercueil wrote: > > Hi Sascha, Greg, > > > > I have a breakage in 6.2-rc* that I eventually bisected to this > > commit, > > on a Ingenic SoC (using the jz4740 musb driver) with ECM or RNDIS > > configured through gadgetfs. > > > > When plugging the board to my PC, the USB network interface is > > recognized, but 'ip link' sees it as 'NO-CARRIER'. With this commit > > reverted on v6.2-rc5, everything works fine. > > Ick, that's not good. Can you send a revert for this? Sascha, any > ideas? Yes, I was hoping Sascha would have a better idea, but I'll send a revert tomorrow. -Paul
Hi Paul, On Wed, Feb 01, 2023 at 01:32:51PM +0000, Paul Cercueil wrote: > Hi Sascha, Greg, > > I have a breakage in 6.2-rc* that I eventually bisected to this commit, > on a Ingenic SoC (using the jz4740 musb driver) with ECM or RNDIS > configured through gadgetfs. > > When plugging the board to my PC, the USB network interface is > recognized, but 'ip link' sees it as 'NO-CARRIER'. With this commit > reverted on v6.2-rc5, everything works fine. I don't have this hardware available. I just tried with a i.MX hardware and it works as expected. I have no idea where the jz4740 musb could behave differently. Here's exactly what I did: mkdir -p /sys/kernel/config/usb_gadget/mygadget cd /sys/kernel/config/usb_gadget/mygadget mkdir -p configs/c.1/strings/0x409 echo "C1:Composite Device" > configs/c.1/strings/0x409/configuration mkdir -p functions/ecm.usb0 ln -s functions/ecm.usb0 configs/c.1/ echo "ci_hdrc.0" > UDC Did you do something differently apart from the "ci_hdrc.0" of course? BTW when you say 'NO-CARRIER' is it on the PC side, board side, or both? It would be great if we could sort this out, but if not I am fine with reverting this patch. I guess this topic will come back to my desk sooner or later then Sascha
Hi Sascha, Le jeudi 09 février 2023 à 11:18 +0100, Sascha Hauer a écrit : > Hi Paul, > > On Wed, Feb 01, 2023 at 01:32:51PM +0000, Paul Cercueil wrote: > > Hi Sascha, Greg, > > > > I have a breakage in 6.2-rc* that I eventually bisected to this > > commit, > > on a Ingenic SoC (using the jz4740 musb driver) with ECM or RNDIS > > configured through gadgetfs. > > > > When plugging the board to my PC, the USB network interface is > > recognized, but 'ip link' sees it as 'NO-CARRIER'. With this commit > > reverted on v6.2-rc5, everything works fine. > > I don't have this hardware available. I just tried with a i.MX > hardware > and it works as expected. I have no idea where the jz4740 musb could > behave differently. > > Here's exactly what I did: > > mkdir -p /sys/kernel/config/usb_gadget/mygadget > cd /sys/kernel/config/usb_gadget/mygadget > mkdir -p configs/c.1/strings/0x409 > echo "C1:Composite Device" > configs/c.1/strings/0x409/configuration > mkdir -p functions/ecm.usb0 > ln -s functions/ecm.usb0 configs/c.1/ > echo "ci_hdrc.0" > UDC > > Did you do something differently apart from the "ci_hdrc.0" of > course? Nothing very different, no. I do: cd /sys/kernel/config/usb_gadget mkdir mtp \ mtp/strings/0x409 \ mtp/configs/c.1 \ mtp/configs/c.1/strings/0x409 \ mtp/functions/ffs.mtp \ mtp/functions/ecm.net \ mtp/functions/rndis.net echo 0x80 > mtp/configs/c.1/bmAttributes echo 500 > mtp/configs/c.1/MaxPower echo 0x049f > mtp/idVendor echo 0x505a > mtp/idProduct echo cdc > mtp/configs/c.1/strings/0x409/configuration ln -s mtp/functions/ecm.net mtp/configs/c.1/ecm.net echo ci_hdrc.0 > mtp/UDC > BTW when you say 'NO-CARRIER' is it on the PC side, board side, or > both? PC side. I don't know what it says on the board side, I can't telnet/SSH. > It would be great if we could sort this out, but if not I am fine > with > reverting this patch. I guess this topic will come back to my desk > sooner or later then Considering that the clock is ticking, let's revert it for now; that will give me some time to debug the issue, and then we can work on a revised patch. Cheers, -Paul
On Thu, Feb 09, 2023 at 10:37:05AM +0000, Paul Cercueil wrote: > Hi Sascha, > > Le jeudi 09 février 2023 à 11:18 +0100, Sascha Hauer a écrit : > > Hi Paul, > > > > On Wed, Feb 01, 2023 at 01:32:51PM +0000, Paul Cercueil wrote: > > > Hi Sascha, Greg, > > > > > > I have a breakage in 6.2-rc* that I eventually bisected to this > > > commit, > > > on a Ingenic SoC (using the jz4740 musb driver) with ECM or RNDIS > > > configured through gadgetfs. > > > > > > When plugging the board to my PC, the USB network interface is > > > recognized, but 'ip link' sees it as 'NO-CARRIER'. With this commit > > > reverted on v6.2-rc5, everything works fine. > > > > I don't have this hardware available. I just tried with a i.MX > > hardware > > and it works as expected. I have no idea where the jz4740 musb could > > behave differently. > > > > Here's exactly what I did: > > > > mkdir -p /sys/kernel/config/usb_gadget/mygadget > > cd /sys/kernel/config/usb_gadget/mygadget > > mkdir -p configs/c.1/strings/0x409 > > echo "C1:Composite Device" > configs/c.1/strings/0x409/configuration > > mkdir -p functions/ecm.usb0 > > ln -s functions/ecm.usb0 configs/c.1/ > > echo "ci_hdrc.0" > UDC > > > > Did you do something differently apart from the "ci_hdrc.0" of > > course? > > Nothing very different, no. > > I do: > > cd /sys/kernel/config/usb_gadget > mkdir mtp \ > mtp/strings/0x409 \ > mtp/configs/c.1 \ > mtp/configs/c.1/strings/0x409 \ > mtp/functions/ffs.mtp \ > mtp/functions/ecm.net \ > mtp/functions/rndis.net > > echo 0x80 > mtp/configs/c.1/bmAttributes > echo 500 > mtp/configs/c.1/MaxPower > > echo 0x049f > mtp/idVendor > echo 0x505a > mtp/idProduct > echo cdc > mtp/configs/c.1/strings/0x409/configuration > ln -s mtp/functions/ecm.net mtp/configs/c.1/ecm.net > > echo ci_hdrc.0 > mtp/UDC > > > BTW when you say 'NO-CARRIER' is it on the PC side, board side, or > > both? > > PC side. I don't know what it says on the board side, I can't > telnet/SSH. I just checked on the host side: With or without my patch I get NO-CARRIER on the host. I have to do a 'ip link set usb0 up' on the device side, with that I get a <BROADCAST,MULTICAST,UP,LOWER_UP> on the host side. Could it be that my patch breaks something on the device side that prevents the device from bringing the link up? Sascha
On Thu, Feb 09, 2023 at 12:41:03PM +0100, Sascha Hauer wrote: > I just checked on the host side: With or without my patch I get > NO-CARRIER on the host. I have to do a 'ip link set usb0 up' on > the device side, with that I get a <BROADCAST,MULTICAST,UP,LOWER_UP> > on the host side. > > Could it be that my patch breaks something on the device side that > prevents the device from bringing the link up? Sascha: When you first posted your original patch, I wondered if it was really the right thing to do. Making the net device not be a child of the UDC device means you can (in theory) have strange behavior such as the kernel suspending the USB device controller while expecting the network interface to keep on working. Is there a different way of solving the original problem? Alan Stern
On Thu, Feb 09, 2023 at 10:05:35AM -0500, Alan Stern wrote: > On Thu, Feb 09, 2023 at 12:41:03PM +0100, Sascha Hauer wrote: > > I just checked on the host side: With or without my patch I get > > NO-CARRIER on the host. I have to do a 'ip link set usb0 up' on > > the device side, with that I get a <BROADCAST,MULTICAST,UP,LOWER_UP> > > on the host side. > > > > Could it be that my patch breaks something on the device side that > > prevents the device from bringing the link up? > > Sascha: > > When you first posted your original patch, I wondered if it was really > the right thing to do. Making the net device not be a child of the UDC > device means you can (in theory) have strange behavior such as the > kernel suspending the USB device controller while expecting the network > interface to keep on working. > > Is there a different way of solving the original problem? I don't know which. One thing would be to couple the lifetime of the ethernet device to the lifetime of the UDC, but the result would look different to userspace, so wouldn't be ideal either. Note the original reason doing this change was that we saw backtraces when doing a 'reboot -f', the 'rmmod dummy_hcd' was just an easy reproducer for the problem. One other possibility might be to take a reference to the UDC while it is in use so that the module can't be rmmoded. Not sure if that fixes my original problem though. Sascha
On Fri, Feb 10, 2023 at 03:49:41PM +0100, Sascha Hauer wrote: > On Thu, Feb 09, 2023 at 10:05:35AM -0500, Alan Stern wrote: > > Sascha: > > > > When you first posted your original patch, I wondered if it was really > > the right thing to do. Making the net device not be a child of the UDC > > device means you can (in theory) have strange behavior such as the > > kernel suspending the USB device controller while expecting the network > > interface to keep on working. > > > > Is there a different way of solving the original problem? > > I don't know which. One thing would be to couple the lifetime of the > ethernet device to the lifetime of the UDC, but the result would look > different to userspace, so wouldn't be ideal either. > > Note the original reason doing this change was that we saw backtraces > when doing a 'reboot -f', the 'rmmod dummy_hcd' was just an easy > reproducer for the problem. > > One other possibility might be to take a reference to the UDC while > it is in use so that the module can't be rmmoded. Not sure if that fixes > my original problem though. Not being familiar with the networking code, I don't really understand the original problem. Does the use-after-free error occur when you try to dereference a dev->parent pointer in the ethernet device? If that's so, then taking a reference (i.e. get_device()) on the parent device should fix the problem. If not, maybe you can give a more detailed guide as to what's going wrong. Alan Stern
On Fri, Feb 10, 2023 at 10:45:54AM -0500, Alan Stern wrote: > On Fri, Feb 10, 2023 at 03:49:41PM +0100, Sascha Hauer wrote: > > On Thu, Feb 09, 2023 at 10:05:35AM -0500, Alan Stern wrote: > > > Sascha: > > > > > > When you first posted your original patch, I wondered if it was really > > > the right thing to do. Making the net device not be a child of the UDC > > > device means you can (in theory) have strange behavior such as the > > > kernel suspending the USB device controller while expecting the network > > > interface to keep on working. > > > > > > Is there a different way of solving the original problem? > > > > I don't know which. One thing would be to couple the lifetime of the > > ethernet device to the lifetime of the UDC, but the result would look > > different to userspace, so wouldn't be ideal either. > > > > Note the original reason doing this change was that we saw backtraces > > when doing a 'reboot -f', the 'rmmod dummy_hcd' was just an easy > > reproducer for the problem. > > > > One other possibility might be to take a reference to the UDC while > > it is in use so that the module can't be rmmoded. Not sure if that fixes > > my original problem though. > > Not being familiar with the networking code, I don't really understand > the original problem. Does the use-after-free error occur when you try > to dereference a dev->parent pointer in the ethernet device? > > If that's so, then taking a reference (i.e. get_device()) on the parent > device should fix the problem. > > If not, maybe you can give a more detailed guide as to what's going > wrong. I don't remember the details anymore. I'll do some more investigation next week. Sascha
Linux regression tracking (Thorsten Leemhuis)
Feb. 16, 2023, 11:07 a.m. UTC |
#12
Addressed
Unaddressed
[TLDR: This mail in primarily relevant for Linux regression tracking. A change or fix related to the regression discussed in this thread was posted or applied, but it did not use a Link: tag to point to the report, as Linus and the documentation call for. Things happen, no worries -- but now the regression tracking bot needs to be told manually about the fix. See link in footer if these mails annoy you.] On 03.02.23 13:46, Linux kernel regression tracking (#adding) wrote: > On 01.02.23 14:32, Paul Cercueil wrote: >> Hi Sascha, Greg, >> >> I have a breakage in 6.2-rc* that I eventually bisected to this commit, >> on a Ingenic SoC (using the jz4740 musb driver) with ECM or RNDIS >> configured through gadgetfs. >> >> When plugging the board to my PC, the USB network interface is >> recognized, but 'ip link' sees it as 'NO-CARRIER'. With this commit >> reverted on v6.2-rc5, everything works fine. > > [CCing the regression list, as it should be in the loop for regressions: > https://docs.kernel.org/admin-guide/reporting-regressions.html] > > Thanks for the report. To be sure the issue doesn't fall through the > cracks unnoticed, I'm adding it to regzbot, the Linux kernel regression > tracking bot: > > #regzbot ^introduced 321b59870f850 > #regzbot title usb: gadget: USB network interface is recognized, but 'ip > link' sees it as 'NO-CARRIER' > #regzbot ignore-activity #regzbot fix: bb07bd68fa0983e3915 #regzbot ignore-activity Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr That page also explains what to do if mails like this annoy you. #regzbot ignore-activity
diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c index e06022873df16..8f12f3f8f6eeb 100644 --- a/drivers/usb/gadget/function/u_ether.c +++ b/drivers/usb/gadget/function/u_ether.c @@ -798,7 +798,6 @@ struct eth_dev *gether_setup_name(struct usb_gadget *g, net->max_mtu = GETHER_MAX_MTU_SIZE; dev->gadget = g; - SET_NETDEV_DEV(net, &g->dev); SET_NETDEV_DEVTYPE(net, &gadget_type); status = register_netdev(net); @@ -873,8 +872,6 @@ int gether_register_netdev(struct net_device *net) struct usb_gadget *g; int status; - if (!net->dev.parent) - return -EINVAL; dev = netdev_priv(net); g = dev->gadget; @@ -905,7 +902,6 @@ void gether_set_gadget(struct net_device *net, struct usb_gadget *g) dev = netdev_priv(net); dev->gadget = g; - SET_NETDEV_DEV(net, &g->dev); } EXPORT_SYMBOL_GPL(gether_set_gadget);