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 X-Spam-Level: X-Spam-Status: No, score=-16.4 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 951FAC433ED for ; Fri, 21 May 2021 08:03:57 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 78FD06135B for ; Fri, 21 May 2021 08:03:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229652AbhEUIFQ (ORCPT ); Fri, 21 May 2021 04:05:16 -0400 Received: from mail.kernel.org ([198.145.29.99]:45362 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236322AbhEUIEs (ORCPT ); Fri, 21 May 2021 04:04:48 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id D1203600CC; Fri, 21 May 2021 08:03:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1621584205; bh=a0t3WD4UC27EPy2xUu8ScpsD4LiO+xkj3W79qcaz3X8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=OOzN/y70CRZNSXi6VI6Ksldu9sgSf3FJBA6C3R30dZPVScVkstAh6qEduDQXwThoh VDKGNz4eZMFyJCv0WFInfTV5QOtpyLt8f76IGtC4zcbEzqhiYo9ziRYeb3DoFvgMRc zwwBX+ka6d9LcJ5lxuHHZX8MHi40zwQkAb7ebylvy47zDiLgIUL2qc2xI0OEHvYjL7 AWnIPZkADAKqyq3I8vbP1rOQi4fAjq6sqeKBSxgyXTslhyiKHfiZ2NqLtZ9QnMtbrb g5oH+ogIvlAcvPLHRBDmyu97CScMzuZbtiekxcpzy5UOzypFoPqlcwitCIGDB15zir 5dgAvwnQfi0sw== Received: from johan by xi.lan with local (Exim 4.94.2) (envelope-from ) id 1lk07y-0004xo-3o; Fri, 21 May 2021 10:03:26 +0200 Date: Fri, 21 May 2021 10:03:26 +0200 From: Johan Hovold To: Alan Stern Cc: Greg KH , "Geoffrey D. Bennett" , USB mailing list Subject: Re: [PATCH] USB: core: WARN if pipe direction != setup packet direction Message-ID: References: <20210520202056.GB1216852@rowland.harvard.edu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210520202056.GB1216852@rowland.harvard.edu> Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org On Thu, May 20, 2021 at 04:20:56PM -0400, Alan Stern wrote: > When a control URB is submitted, the direction indicated by URB's pipe > member is supposed to match the direction indicated by the setup > packet's bRequestType member. A mismatch could lead to trouble, > depending on which field the host controller drivers use for > determining the actual direction. > > This shouldn't ever happen; it would represent a careless bug in a > kernel driver somewhere. This patch adds a dev_WARN to let people > know about the potential problem. > > Suggested-by: "Geoffrey D. Bennett" > Signed-off-by: Alan Stern > > --- > > > [as1960] > > > drivers/usb/core/urb.c | 3 +++ > 1 file changed, 3 insertions(+) > > Index: usb-devel/drivers/usb/core/urb.c > =================================================================== > --- usb-devel.orig/drivers/usb/core/urb.c > +++ usb-devel/drivers/usb/core/urb.c > @@ -407,6 +407,9 @@ int usb_submit_urb(struct urb *urb, gfp_ > return -ENOEXEC; > is_out = !(setup->bRequestType & USB_DIR_IN) || > !setup->wLength; > + if (usb_pipeout(urb->pipe) != is_out) > + dev_WARN(&dev->dev, "BOGUS control dir, pipe %x doesn't match bRequestType %x\n", > + urb->pipe, setup->bRequestType); > } else { > is_out = usb_endpoint_dir_out(&ep->desc); > } While I agree with intention here, I'm worried that this will start flooding the logs of users. So first, this should probably be rate limited. Second, did you try to estimate how many call sites that get this wrong? I always felt a bit pedantic when pointing out that the pipe direction should match the request type to driver author's during review when (in almost all cases?) this hasn't really mattered. I fear we may have accumulated a fairly large number of these mismatches over the years but I haven't verified that. Johan