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 026DDC4332F for ; Wed, 23 Nov 2022 13:24:55 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 6B7A46B0071; Wed, 23 Nov 2022 08:24:55 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 667466B0073; Wed, 23 Nov 2022 08:24:55 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 557926B0074; Wed, 23 Nov 2022 08:24:55 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 45FEA6B0071 for ; Wed, 23 Nov 2022 08:24:55 -0500 (EST) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id F1D0E40E4B for ; Wed, 23 Nov 2022 13:24:54 +0000 (UTC) X-FDA: 80164777308.13.15E16D4 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf02.hostedemail.com (Postfix) with ESMTP id B4C078000A for ; Wed, 23 Nov 2022 13:24:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=a6cCVVEt16pUp7IjlR7FRGcFelPEQyVp+VfuhVmOVlg=; b=bkJDnu9YsW/TxUA5ZAKnDeGihE LR6xDjQEfhfVS/zC3bZ+14rUJ/3wksjcPhAdVyYLEGjRco2ZJDBkZQ8ZgOgx6qd4RTAfskW8hciVi QOJZUFhtW+A08gnqMmPA9Nlv8/SDHlrg2MVbeAwUde0/rVI10Mzx536D5Xb64n62WLQrOyjGXfZ/v GIF7rJUERrMXhYc3QjKgoNFV77LcrDCfnZZjFodyCjk0euk5vwTGgcEXXaZsVERP9Plrv6vhYI0jy 16pyOupGb7wMTaH7p4Mgwm25dD56tC5DefFfK7qjIEZTt24FIFg6ufMr8CmYHyxTblRLgTleaNfIz V9M0L+kA==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1oxpkB-007d5i-JS; Wed, 23 Nov 2022 13:24:51 +0000 Date: Wed, 23 Nov 2022 13:24:51 +0000 From: Matthew Wilcox To: Baoquan He Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, akpm@linux-foundation.org, stephen.s.brennan@oracle.com, urezki@gmail.com, hch@infradead.org Subject: Re: [PATCH RFC 3/3] mm/vmalloc.c: allow vread() to read out vm_map_ram areas Message-ID: References: <20221109033535.269229-1-bhe@redhat.com> <20221109033535.269229-4-bhe@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1669209894; a=rsa-sha256; cv=none; b=G+d37suLlEEyAKHJVIA2m4tPad6euudWBrKfSfxOBrshOK1ojhE0xIhQBeymnQ6gBN+71v hyOSYa91AovRin+8RmBqgeWEoL3N4Kbb/5K1L56YD3AglC0kHxZ04v3XTwfzrhU/njTb8G 2NW/UT0V99Nq7ssnkg1JtjTVLRNltgc= ARC-Authentication-Results: i=1; imf02.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=bkJDnu9Y; dmarc=none; spf=none (imf02.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1669209894; 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:dkim-signature; bh=a6cCVVEt16pUp7IjlR7FRGcFelPEQyVp+VfuhVmOVlg=; b=HhyEQuulTt+7xaoj6jPNldHzXiSs/u7vBikmdFf2vs3EraA8eFfJ59AkRaWin2H5GPBTKE czVHYCYlBf+6JF8AqFGU495Z51oaKRnij84PHus/InOe8KzBkfE1EDj2hvmRyNPZpk1d2v lojVjToykv1ZljJKFTbwyfNmR3aVVVQ= X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: B4C078000A X-Rspam-User: Authentication-Results: imf02.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=bkJDnu9Y; dmarc=none; spf=none (imf02.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org X-Stat-Signature: riayxdrdm1hexsmg9xziuo5aom8dq3w6 X-HE-Tag: 1669209892-929276 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, Nov 23, 2022 at 11:38:54AM +0800, Baoquan He wrote: > On 11/18/22 at 08:01am, Matthew Wilcox wrote: > > On Wed, Nov 09, 2022 at 11:35:34AM +0800, Baoquan He wrote: > > > Currently, vread() can read out vmalloc areas which is associated with > > > a vm_struct. While this doesn't work for areas created by vm_map_ram() > > > interface because it doesn't allocate a vm_struct. Then in vread(), > > > these areas will be skipped. > > > > > > Here, add a new function vb_vread() to read out areas managed by > > > vmap_block specifically. Then recognize vm_map_ram areas via vmap->flags > > > and handle them respectively. > > > > i don't understand how this deals with the original problem identified, > > that the vread() can race with an unmap. > > Thanks for checking. > > I wrote a paragraph, then realized I misunderstood your concern. You are > saying the comment from Uladzislau about my original draft patch, right? > Paste the link of Uladzislau's reply here in case other people want to > know the background: > https://lore.kernel.org/all/Y1uKSmgURNEa3nQu@pc636/T/#u > > When Stephen raised the issue originally, I posted a draft patch as > below trying to fix it: > https://lore.kernel.org/all/Y1pHTj2wuhoWmeV3@MiWiFi-R3L-srv/T/#u > > In above draft patch, I tried to differentiate normal vmalloc area and > vm_map_ram area with the fact that vmalloc area is associated with a > vm_struct, while vm_map_ram area has ->vm as NULL. And I thought their > only difference is normal vmalloc area has guard page, so its size need > consider the guard page; while vm_map_ram area has no guard page, only > consider its own actual size. Uladzislau's comment reminded me I was > wrong. And the things we need handle are beyond that. > > Currently there are three kinds of vmalloc areas in kernel: > > 1) normal vmalloc areas, associated with a vm_struct, this is allocated > in __get_vm_area_node(). When freeing, it set ->vm to NULL > firstly, then unmap and free vmap_area, see remove_vm_area(). > > 2) areas allocated via vm_map_ram() and size is larger than > VMAP_MAX_ALLOC. The entire area is not associated with vm_struct, and > freed at one time in vm_unmap_ram() with unmapping and freeing vmap_area; > > 3) areas allocated via vm_map_ram(), then delegate to vb_alloc() when > size <= VMAP_MAX_ALLOC. Its vmap_area is allocated at one time with > VMAP_BLOCK_SIZE big, and split and used later through vb_alloc(), freed > via vb_free(). When the entire area is dirty, it will be unmapped and > freed. > > Based on above facts, we need add flags to differentiate the normal > vmalloc area from the vm_map_ram area, namely area 1) and 2). And we > also need flags to differentiate the area 2) and 3). Because area 3) are > pieces of a entire vmap_area, vb_free() will unmap the piece of area and > set the part dirty, but the entire vmap_area will kept there. So when we > will read area 3), we need take vb->lock and only read out the still > mapped part, but not dirty or free part of the vmap_area. I don't think you understand the problem. Task A: Task B: Task C: p = vm_map_ram() vread(p); ... preempted ... vm_unmap_ram(p); q = vm_map_ram(); vread continues If C has reused the address space allocated by A, task B is now reading the memory mapped by task C instead of task A. If it hasn't, it's now trying to read from unmapped, and quite possibly freed memory. Which might have been allocated by task D. Unless there's some kind of reference count so that B knows that both the address range and the underlying memory can't be freed while it's in the middle of the vread(), this is just unsafe.