From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750984AbdE1QDf (ORCPT ); Sun, 28 May 2017 12:03:35 -0400 Received: from sauhun.de ([88.99.104.3]:51860 "EHLO pokefinder.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750811AbdE1QDd (ORCPT ); Sun, 28 May 2017 12:03:33 -0400 Date: Sun, 28 May 2017 18:03:30 +0200 From: Wolfram Sang To: Florian Fainelli Cc: linux-kernel@vger.kernel.org, Greg Kroah-Hartman , Felipe Balbi , Peter Chen , Roger Quadros , Alan Stern , Mathias Nyman , Javier Martinez Canillas , Baoyou Xie , Sekhar Nori , William wu , Arnd Bergmann , Chris Bainbridge , Wolfram Sang , Krzysztof Opasiak , Felix =?utf-8?Q?H=C3=A4dicke?= , Colin Ian King , "open list:USB SUBSYSTEM" , clemens@ladisch.de, maksim.salau@gmail.com Subject: Re: [PATCH v3 0/2] usb: Check for DMA capable buffer sanity Message-ID: <20170528160330.xjseehf6nnoooe75@ninjato> References: <20170426005612.24850-1-f.fainelli@gmail.com> <84087586-d5fc-3115-4b16-93c75e411c59@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="gfhturhmb3d4ethb" Content-Disposition: inline In-Reply-To: <84087586-d5fc-3115-4b16-93c75e411c59@gmail.com> User-Agent: NeoMutt/20170113 (1.7.2) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --gfhturhmb3d4ethb Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, May 05, 2017 at 02:08:31PM -0700, Florian Fainelli wrote: > On 04/25/2017 05:56 PM, Florian Fainelli wrote: > > Changes in v3: > >=20 > > - added check in usb_gadget_map_request_by_dev (Felipe), new patch > > - improved commit message description (Clemens) > > - added additiona checks for urb->setup_packet (Alan) > >=20 > > Changes in v2: > >=20 > > - moved the check from usb_start_wait_urb() to usb_hcd_map_urb_for_dma() >=20 > Is this version looking good now? Thanks! So, it seems I am in a similar situation with the I2C subsystem right now. I need to check the message buffers if they are DMA capable. Because you have basically the same checks in 3 different places, and I need something similar for I2C, I wondered about a generic place to put these checks. Especially since we want future improvements to these checks applied everywhere immediately. Here is a small diff on what I have now: =3D=3D=3D dma-mapping(?): introduce helper to check for DMA capable addresses Introduce a helper to check if an address is DMA capable. Such a check is subtle, so it is good to have a centralized place for it. Note: I am absolutely not sure if dma-mapping.h is a good place for such a function. I just couldn't think of a better one for now. Second note: I am not even sure the checks are complete (kmapped mem?). I am not an MM expert. But that just strengthens the argument of having on centralized place IMO. Signed-off-by: Wolfram Sang diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 5dea98358c05c4..777a37b395ff19 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -1584,7 +1584,7 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, stru= ct urb *urb, ret =3D -EAGAIN; else urb->transfer_flags |=3D URB_DMA_MAP_PAGE; - } else if (is_vmalloc_addr(urb->transfer_buffer)) { + } else if (!is_dma_capable_addr(urb->transfer_buffer)) { WARN_ONCE(1, "transfer buffer not dma capable\n"); ret =3D -EAGAIN; } else { diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h index 4f3eecedca2d7c..da8c1230302505 100644 --- a/include/linux/dma-mapping.h +++ b/include/linux/dma-mapping.h @@ -10,6 +10,8 @@ #include #include #include +#include +#include =20 /** * List of possible attributes associated with a DMA mapping. The semantics @@ -818,4 +820,10 @@ static inline int dma_mmap_wc(struct device *dev, #define dma_unmap_len_set(PTR, LEN_NAME, VAL) do { } while (0) #endif =20 +/* only works in process context because of stack detection */ +static inline bool is_dma_capable_addr(void *addr) +{ + return !(is_vmalloc_or_module_addr(addr) || + object_is_on_stack(addr)); +} #endif diff --git a/mm/vmalloc.c b/mm/vmalloc.c index 34a1c3e46ed725..47de0c0a700e7c 100644 --- a/mm/vmalloc.c +++ b/mm/vmalloc.c @@ -261,6 +261,7 @@ int is_vmalloc_or_module_addr(const void *x) #endif return is_vmalloc_addr(x); } +EXPORT_SYMBOL_GPL(is_vmalloc_or_module_addr); =20 /* * Walk a vmap address to the struct page it maps. =3D=3D=3D The WIP branch containing also the I2C parts can be found here: git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/topic/i= 2c-core-dma I think the whole series needs 1 or 2 days more before I send out an RFC, but I thought I'll let you know about my idea already. Thanks and kind regards, Wolfram --gfhturhmb3d4ethb Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEEOZGx6rniZ1Gk92RdFA3kzBSgKbYFAlkq9M4ACgkQFA3kzBSg Kba6CQ/+MEn/bkLqWySwm9sMjs8MPMhFhmJwg/FKgnMYENnrBUv0sXBEX2/ErVJC vrlh9kiriLNboA3JCnAuGBEOHUklyjM1eJkaRXwpFVT9l4JxnbDCGcjtxja68gmG F5tYdxt6s6fDuQo+MfSCZl4Lynd26723lTDXjquhgehFQQ160Q4XrZm7ZzkXXTOk /gkrrIcP3UQxs4CO5TKoiVcH9geCy8Qg/N/u+fwHXrreeUBd5wGkwtNfI0AFIrPA 3bj9fNYkBGvcwS291uHrE75iDxmG9AflWZEihvPaMwPRrEwZBH9s0ZgzUIEO8ipl H0D2aUrRYHf0RgLzbrIAvpo8OA/u9zumpnX5BVa5wmyoY71FMDMQDBTpgKcWWQYY d4xLZk+NZwXgcL1on+Enaz3nLa+IFZi4eO8A9dsVDydOjRcR8+MOCrtMATi+vCVH q3e2rxS17bT+1+ppp0IeH4i2QEnDyiLpBfeyjpUC4lTVh9mjQ4Co1HrfiNsrLSpy Ldl0E8UPcOXp2Mnz1lAVYywEdOlHfY1d4bYE4VE1G2VneTw7NkGDESiWJ5460LY8 ors9vfhtjCxRfunfTg/Yg0w7pUbCD55kISkUa5tfA14cOotvdBjN3g4qv856/JMl Rq1n+4+CCpUeecXhFOrygnZWjIEAS/aPx1PlYBntFxAWP9df7M4= =S0I7 -----END PGP SIGNATURE----- --gfhturhmb3d4ethb--