From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id D62FEEB64D7 for ; Tue, 20 Jun 2023 11:20:36 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 5F6E18621C; Tue, 20 Jun 2023 13:20:34 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=tinet.cat Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: by phobos.denx.de (Postfix, from userid 109) id EC60F861CE; Tue, 20 Jun 2023 13:20:32 +0200 (CEST) Received: from mx1.tinet.cat (mx1.dipta.cat [195.76.233.59]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 6CD5C86259 for ; Tue, 20 Jun 2023 13:20:28 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=tinet.cat Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=xdrudis@tinet.cat X-ASG-Debug-ID: 1687260027-12aaf2530b1809f0001-4l7tJC Received: from smtp01.tinet.cat (smtp01.tinet.org [195.77.216.131]) by mx1.tinet.cat with ESMTP id nFcYGrlyJbp1ao94; Tue, 20 Jun 2023 13:20:27 +0200 (CEST) X-Barracuda-Envelope-From: xdrudis@tinet.cat X-Barracuda-Effective-Source-IP: smtp01.tinet.org[195.77.216.131] X-Barracuda-Apparent-Source-IP: 195.77.216.131 Received: from xdrudis.tinet.cat (180.red-79-152-181.dynamicip.rima-tde.net [79.152.181.180]) by smtp01.tinet.cat (Postfix) with ESMTPSA id EA352605DE48; Tue, 20 Jun 2023 13:20:26 +0200 (CEST) Date: Tue, 20 Jun 2023 13:20:25 +0200 From: Xavier Drudis Ferran To: Simon Glass Cc: Xavier Drudis Ferran , Marek Vasut , u-boot@lists.denx.de, Lukasz Majewski , Sean Anderson Subject: Re: [SPAM] Re: [PATCH] cmd: usb: Prevent reset in usb tree/info command Message-ID: X-ASG-Orig-Subj: Re: [SPAM] Re: [PATCH] cmd: usb: Prevent reset in usb tree/info command References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: X-Barracuda-Connect: smtp01.tinet.org[195.77.216.131] X-Barracuda-Start-Time: 1687260027 X-Barracuda-URL: https://webmail.tinet.cat:443/cgi-mod/mark.cgi X-Barracuda-Scan-Msg-Size: 2395 X-Barracuda-BRTS-Status: 1 X-Barracuda-Bayes: INNOCENT GLOBAL 0.5131 1.0000 0.7500 X-Barracuda-Spam-Score: 0.75 X-Barracuda-Spam-Status: No, SCORE=0.75 using global scores of TAG_LEVEL=1000.0 QUARANTINE_LEVEL=6.0 KILL_LEVEL=8.0 tests= X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.110296 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.8 at phobos.denx.de X-Virus-Status: Clean El Tue, Jun 20, 2023 at 11:03:57AM +0100, Simon Glass deia: > Hi Xavier, > Hi Simon, > > > > It is also possible that one day a device that is not UCLASS_BLK, > > UCLASS_BOOTDEV or UCLASS_USB_EMUL is put as children of a usb storage > > device (just imagine a future system similar to bootstd for firmware > > updates, trust material, etc.). Is it likely to have a struct in > > parent_priv_ that is not a usb_device ? > > > > So which is more likely to survive future changes ? > > > > - checking for parent_priv_ not null and not UCLASS_USB_EMUL > > > > - checking for parent_priv_ not null and not UCLASS_USB_EMUL and not UCLASS_BLK > > (my patch, overcautious ?) > > > > - checking for not (UCLASS_BLK, UCLASS_BOOTDEV or UCLASS_USB_EMUL) > > (Simon Glass' idea) > > > > - checking for not UCLASS_BLK and not UCLASS_BOOTDEV and not UCLASS_USB_EMUL > > and parent_priv_ not null > > Really the parent_priv thing is a separate issue, a side effect of > something having a UCLASS_USB parent. > I don't think it's a separate issue. If parent_priv is present it could be a usb_device (most likely) or not, but if it's null there's no way the recursive call can succeed. > The key point here is that we cannot iterate down into a bootdev > device looking for USB devices. So we should use that as the check, > since it is the most direct check. > But things keep appearing that have a UCLASS_USB* parent and no parent_priv. in 2017 Suneel Garapati already fixed the issue of UCLASS_BLK being a child of a device. Now it's UCLASS_BOOTDEV, and tomorrow may be something else. The most direct check will miss future cases as the devices tend to become more abstract instead of mapping one to one to physical stuff. > > >From my memory, I think you can check for a USB hub instead, but I'm > not completely sure. > On second thoughts I didn't find it so easy. There's the root hub, UCLASS_USB, I think and a UCLASS_USB_EMUL may also be a hub, so I don't know anymore how more elegant that could be, so I left it be. > I suggest adding a test for the command (see test/dm/acpi.c for an > example) since a test is the best way to ensure this doesn't happen > again. > Makes sense. But I don't have any more time for that, sorry. I think we'll have to leave it at this unless someone else has the time. Bye.