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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5476BC7EE22 for ; Wed, 10 May 2023 14:37:50 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C59326B0071; Wed, 10 May 2023 10:37:49 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id BE1A86B0072; Wed, 10 May 2023 10:37:49 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A82716B0074; Wed, 10 May 2023 10:37:49 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 9516E6B0071 for ; Wed, 10 May 2023 10:37:49 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 46B281A04D8 for ; Wed, 10 May 2023 14:37:49 +0000 (UTC) X-FDA: 80774599458.05.BB11CA5 Received: from netrider.rowland.org (netrider.rowland.org [192.131.102.5]) by imf24.hostedemail.com (Postfix) with SMTP id 90645180009 for ; Wed, 10 May 2023 14:37:46 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=none; spf=pass (imf24.hostedemail.com: domain of stern+6456d096@netrider.rowland.org designates 192.131.102.5 as permitted sender) smtp.mailfrom=stern+6456d096@netrider.rowland.org; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1683729466; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=QpqeSXebkxMjm8UKxpRNEJM4CYRIubnRV3k1OInz4R4=; b=Q7LGsz/ecwTYxxtZlOlXT2Ql5NRRBvmOj8Hn874Lxf0RhEz/+iTLxs9r7aQ9Pc0nyJExdD 2En5tglrCwfjTJI0EKBDRWLHhr+2wl5AkhKOduYi47jBnQnuEyBImLt+8R1GUZvd3YaLhq Tigx5k9hbYQE2lZHMGmexzMlInqIXs0= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=none; spf=pass (imf24.hostedemail.com: domain of stern+6456d096@netrider.rowland.org designates 192.131.102.5 as permitted sender) smtp.mailfrom=stern+6456d096@netrider.rowland.org; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1683729466; a=rsa-sha256; cv=none; b=f6w7HxJkHUjaVVlmdW1xVJBTMCs7Nb/X2oeccA1eTz6vx7YqlCxvdeo3q+fqvoxyjHdk13 1ng+KHm/on6x6+aFk7OxYv6OSzdfJf5WQjh8rHK94c9LJOUv+qaWtSrr+ERGZEReSZ9bFF BMN1rF9n479HfmiAF/QW+vjsOhXZsmI= Received: (qmail 623576 invoked by uid 1000); 10 May 2023 10:37:45 -0400 Date: Wed, 10 May 2023 10:37:45 -0400 From: Alan Stern To: Ruihan Li Cc: linux-mm@kvack.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, Pasha Tatashin , David Hildenbrand , Matthew Wilcox , Andrew Morton , Christoph Hellwig , Greg Kroah-Hartman , syzbot+fcf1a817ceb50935ce99@syzkaller.appspotmail.comm, stable@vger.kernel.org Subject: Re: [PATCH 1/4] usb: usbfs: Enforce page requirements for mmap Message-ID: <65ae7b7f-9dea-429f-aca6-2ce4a75b6531@rowland.harvard.edu> References: <20230510085527.57953-1-lrh2000@pku.edu.cn> <20230510085527.57953-2-lrh2000@pku.edu.cn> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230510085527.57953-2-lrh2000@pku.edu.cn> X-Stat-Signature: 5kjo8apmbuzs7h3iyidbee64ck7zqmyc X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 90645180009 X-Rspam-User: X-HE-Tag: 1683729466-458059 X-HE-Meta: U2FsdGVkX1/RPJ9LMFx701GttQVhKE/7M3DnjlNa43+NtO4IYywAwN1ffrGFa5X9Kaq8/RWv4w+3QOMal18uHhoj3JpEbYBND0h7laP1XfiywfcZFxcddv0l4XflNLMPzpP/6XwcDwI043gNOhxpg8frmzkJ886VpjCwO0+4+0vgDQfK9bnx8d2Lu8JJICF2HQaMhK2SzP6YbKGEQmKVZY9bIg2TCGSqWMJo9aY9ZcqmUAODW6Ewbu47UP6devn9zPhTgE19V3YF7Ad9srdmLC4ikj0UttEbVMA5R8nJYhSpnpRh5Emrh9dpqiCxiyzHjKaxeOhMS7tCjIgHJa2iQHp170o/yTwVcEVCvV16xERkh4e7Z0FqmCJz9G+Xq2PWjnO+Q3/ThVNkjg2bYs73kt5NcE0w4EToEoDw8pdlgrOxj7yAoo5oAbYka2FrKvLTNNbfinYrpPuSqyFqZeVchKSBTubSMdZ9EtKG3OJ40KqWjD4uM6DdpNtMgUCiYoHY/N5yGBnQNICHA/K5jo05XFZNco/xrFWwtAI5yO27zshH7Q7n834A4c8dWHKz7LNN3XwtNbvoqun2eVk7kt0A9YfyXOPyG7YP57jAD+sFxrSFqB4lLQp87n5wb4obQ8pmS/DMs99wlz1NsfXE3g6uYAJeCpaGW0/HiYfA+Sg3/fN4ogAFDxUzb+RjaME9rAQ43dfg5Va9ptYaEkUADyyJw1mXlhxYr2zdpVl9/U+CbqHIs8rQMZfTggcmnZ+HONbXGK0vpVNBiqWLnhZO8wTMg8f6pgv/S83whRmhwtfY2bI3taYwbunomTP3fyVSCQKF8AM7Mu4BDJLA3q1ODvKfSv7gCLHpWAsHGARgvxAKoMOU6Z3+OTlJNFcfT0CDAyfhvaJSYhs7TvvTBtNbsrmc3VGfaIU9gQ3v4C83XGapyQem2rXwAK7J9awe32LoINou0qJ5SNYouJgS41+I89I OcQBTjyE 4iz0InQuXJjIJ5QRm/QlzlLR/NvZyEFwX56oeRuqQMS2oYK00jI5xhD0kFbYxw5iOMvSKEXq6G+dsxdavZyMWcjfKWL7nnizBIpjLHIi4zryGKzsqp9N2ij58ETul2Q+Lb7w+lmx/AR1U/ZVEUz9PMc8cTvcL7xMzaw9QduvZXUoBhBwn8FWM0GJyZTT/miJr9x/ZTbay9FQ/TGAjiq+6raGYL4pH2XC9spcQFFr0MQSDJCepKsetO4bl6Rfu2WcR5FWnPXWBYjqQ2mxMtPxkUbhUv5x9Z9oz+zp81KNsw1FZNbQcLQI1QfWkMbql0sUAcRacezNVBdBSOuJ3f0ZzwBES6Me90ZQ26yZe+RzfRpEQUmyy4SE1T0PDwFgbvHntYpQTiUbLNyYVqpBwSyGjq2Y8yg== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Wed, May 10, 2023 at 04:55:24PM +0800, Ruihan Li wrote: > The current implementation of usbdev_mmap uses usb_alloc_coherent to > allocate memory pages that will later be mapped into the user space. > Meanwhile, usb_alloc_coherent employs three different methods to > allocate memory, as outlined below: > * If hcd->localmem_pool is non-null, it uses gen_pool_dma_alloc to > allocate memory. > * If DMA is not available, it uses kmalloc to allocate memory. > * Otherwise, it uses dma_alloc_coherent. > > However, it should be noted that gen_pool_dma_alloc does not guarantee > that the resulting memory will be page-aligned. Furthermore, trying to > map slab pages (i.e., memory allocated by kmalloc) into the user space > is not resonable and can lead to problems, such as a type confusion bug > when PAGE_TABLE_CHECK=y [1]. > > To address these issues, this patch introduces hcd_alloc_coherent_pages, > which addresses the above two problems. Specifically, > hcd_alloc_coherent_pages uses gen_pool_dma_alloc_align instead of > gen_pool_dma_alloc to ensure that the memory is page-aligned. To replace > kmalloc, hcd_alloc_coherent_pages directly allocates pages by calling > __get_free_pages. > > Reported-by: syzbot+fcf1a817ceb50935ce99@syzkaller.appspotmail.comm > Closes: https://lore.kernel.org/lkml/000000000000258e5e05fae79fc1@google.com/ [1] > Cc: stable@vger.kernel.org > Signed-off-by: Ruihan Li > --- I'm never quite sure about when it makes sense to complain about stylistic issues. Nevertheless, I'm going to do so here... > drivers/usb/core/buffer.c | 41 +++++++++++++++++++++++++++++++++++++++ > drivers/usb/core/devio.c | 9 +++++---- > include/linux/usb/hcd.h | 5 +++++ > 3 files changed, 51 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c > index fbb087b72..6010ef9f5 100644 > --- a/drivers/usb/core/buffer.c > +++ b/drivers/usb/core/buffer.c > @@ -172,3 +172,44 @@ void hcd_buffer_free( > } > dma_free_coherent(hcd->self.sysdev, size, addr, dma); > } > + > +void *hcd_buffer_alloc_pages(struct usb_hcd *hcd, size_t size, > + gfp_t mem_flags, dma_addr_t *dma) > +{ > + if (size == 0) > + return NULL; > + > + if (hcd->localmem_pool) > + return gen_pool_dma_alloc_align(hcd->localmem_pool, > + size, dma, PAGE_SIZE); C isn't Lisp. Expressions in C are not based entirely around parentheses, and it's not necessary to align our code based on the parenthesized sub-expressions to avoid hopelessly confusing the reader. The style used in this file (and many other places in the USB core) is to indent continuation lines by two tab stops. The same comment applies to all the other continuation lines you added or changed in this patch and in patch 2/4. Alan Stern