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=-0.6 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no 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 4A3D9C34026 for ; Tue, 18 Feb 2020 15:17:51 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id F0C9E207FD for ; Tue, 18 Feb 2020 15:17:50 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=shutemov-name.20150623.gappssmtp.com header.i=@shutemov-name.20150623.gappssmtp.com header.b="PzuGy0rl" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org F0C9E207FD Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=shutemov.name Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 9D6A46B0003; Tue, 18 Feb 2020 10:17:50 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 987536B0006; Tue, 18 Feb 2020 10:17:50 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 89D716B0007; Tue, 18 Feb 2020 10:17:50 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0125.hostedemail.com [216.40.44.125]) by kanga.kvack.org (Postfix) with ESMTP id 73A756B0003 for ; Tue, 18 Feb 2020 10:17:50 -0500 (EST) Received: from smtpin17.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 277AABBCF for ; Tue, 18 Feb 2020 15:17:50 +0000 (UTC) X-FDA: 76503602700.17.rod62_a1b995b61160 X-HE-Tag: rod62_a1b995b61160 X-Filterd-Recvd-Size: 7081 Received: from mail-lj1-f195.google.com (mail-lj1-f195.google.com [209.85.208.195]) by imf32.hostedemail.com (Postfix) with ESMTP for ; Tue, 18 Feb 2020 15:17:49 +0000 (UTC) Received: by mail-lj1-f195.google.com with SMTP id n18so23379649ljo.7 for ; Tue, 18 Feb 2020 07:17:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=shutemov-name.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to; bh=qWhEX9RKxhypnM5PPqWFOUyYSGe+pegPEe4K7fpUEmA=; b=PzuGy0rlY3m0PILFANKBf4jU07YdevfnTGGFHMjs9F/K8E1yFdiUnUyvR7m8no03/h 3pCXn1X+ehHvBcC3Uc3QRYlT/ZjtUhn7I3SiF6yhssc/WTwymLNmQRg8b73Fcm/TzvJd is/qIVvOqL9gFoekZwT0N9mvfC9j0pO39BjX7KBGYqJhCZFbnNX8EIAObEpnQl+Phcx7 zKYzVNXnUnHTB1ejNs/Rv9cu1w0ldFPqJDk7qgMGuFprB1XspaTqLRWZSbLGSb07BOOi 05unZ2B2QATKxlasmD2FOCEjp2WZ0CNF820QeigP/i2HUW6XVE+i6Uv6tjUzfJEQ9K6Q 23Xw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to; bh=qWhEX9RKxhypnM5PPqWFOUyYSGe+pegPEe4K7fpUEmA=; b=cDZaGBvKUOFY28BMpkOTlIHGkOrx9zcWg4aZDFOr9FqJPrJggX9dvjU/wADib0jnwB ZPE6aRezA6y2l9QWAcvcoZ04/BkXRIVvse+ldi2w71VnVaYlBvEWU5yVJl9GyybooImD qIXSJmgVrq5R4SekT/QfRklphIXjEgUXRXXjxXrT5YcNRYNOlXhPbUvQQyVLuX2e8NYt 2VaZ5UgMYgk0pkmaJyCMQjAEAGSc3Nu1VoD10KsOJGOJyVY/jJo/aD6PsO0AJHZkBbgT BVTtf7ahFZkvgTCdJbU38LJphgiKs0pveKYcC8qIdxHo6oXwqOxnoViKSFuA9/Xz/g3F 77Dw== X-Gm-Message-State: APjAAAV0HCRI4t+bo8baS0NBqvls+uL6OrBD5tg6Ncw1VP1l7bW70GFr 3yrdER35ImW7MeB8M2oUSs5mdw== X-Google-Smtp-Source: APXvYqwmDGsCUfLrJ8biBI5KsgtzZAXb+nMk2XFsea2b0gKSENYf0OwiYFWttt/aHElu78KUASDZmg== X-Received: by 2002:a05:651c:239:: with SMTP id z25mr12797943ljn.48.1582039067743; Tue, 18 Feb 2020 07:17:47 -0800 (PST) Received: from box.localdomain ([86.57.175.117]) by smtp.gmail.com with ESMTPSA id g15sm2747705ljk.8.2020.02.18.07.17.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Feb 2020 07:17:47 -0800 (PST) Received: by box.localdomain (Postfix, from userid 1000) id 74291100FA3; Tue, 18 Feb 2020 18:18:13 +0300 (+03) Date: Tue, 18 Feb 2020 18:18:13 +0300 From: "Kirill A. Shutemov" To: Qian Cai Cc: Marco Elver , Andrew Morton , Linux Memory Management List , LKML , Peter Zijlstra , syzbot , syzkaller-bugs Subject: Re: [PATCH -next] fork: annotate a data race in vm_area_dup() Message-ID: <20200218151813.3yzzb2hzlmtbf5xg@box> References: <20200218103002.6rtjreyqjepo3yxe@box> <93E6B243-9A0F-410C-8EE4-9D57E28AF5AF@lca.pw> <1582038035.7365.93.camel@lca.pw> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1582038035.7365.93.camel@lca.pw> Content-Transfer-Encoding: quoted-printable X-Bogosity: Ham, tests=bogofilter, spamicity=0.000030, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, Feb 18, 2020 at 10:00:35AM -0500, Qian Cai wrote: > On Tue, 2020-02-18 at 15:09 +0100, Marco Elver wrote: > > On Tue, 18 Feb 2020 at 13:40, Qian Cai wrote: > > >=20 > > >=20 > > >=20 > > > > On Feb 18, 2020, at 5:29 AM, Kirill A. Shutemov wrote: > > > >=20 > > > > I think I've got this: > > > >=20 > > > > vm_area_dup() blindly copies all fields of orignal VMA to the new= one. > > > > This includes coping vm_area_struct::shared.rb which is normally = protected > > > > by i_mmap_lock. But this is fine because the read value will be > > > > overwritten on the following __vma_link_file() under proper prote= ctection. > > >=20 > > > Right, multiple processes could share the same file-based address s= pace where those vma have been linked into address_space::i_mmap via vm_a= rea_struct::shared.rb. Thus, the reader could see its shared.rb linkage p= ointers got updated by other processes. > > >=20 > > > >=20 > > > > So the fix is correct, but justificaiton is lacking. > > > >=20 > > > > Also, I would like to more fine-grained annotation: marking with > > > > data_race() 200 bytes copy may hide other issues. > > >=20 > > > That is the harder part where I don=E2=80=99t think we have anythin= g for that today. Macro, any suggestions? ASSERT_IGNORE_FIELD()? > >=20 > > There is no nice interface I can think of. All options will just caus= e > > more problems, inconsistencies, or annoyances. > >=20 > > Ideally, to not introduce more types of macros and keep it consistent= , > > ASSERT_EXCLUSIVE_FIELDS_EXCEPT(var, ...) maybe what you're after: > > "Check no concurrent writers to struct, except ignore the provided > > fields". > >=20 > > This option doesn't quite work, unless you just restrict it to 1 fiel= d > > (we can't use ranges, because e.g. vm_area_struct has > > __randomize_layout). The next time around, you'll want 2 fields, and > > it won't work. Also, do we know that 'shared.rb' is the only thing we > > want to ignore? > >=20 > > If you wanted something similar to ASSERT_EXCLUSIVE_BITS, it'd have t= o > > be ASSERT_EXCLUSIVE_FIELDS(var, ...), however, this is quite annoying > > for structs with many fields as you'd have to list all of them. It's > > similar to what you can already do currently (but I also don't > > recommend because it's unmaintainable): > >=20 > > ASSERT_EXCLUSIVE_WRITER(orig->vm_start); > > ASSERT_EXCLUSIVE_WRITER(orig->vm_end); > > ASSERT_EXCLUSIVE_WRITER(orig->vm_next); > > ASSERT_EXCLUSIVE_WRITER(orig->vm_prev); > > ... and so on ... > > *new =3D data_race(*orig); > >=20 > > Also note that vm_area_struct has __randomize_layout, which makes > > using ranges impossible. All in all, I don't see a terribly nice > > option. > >=20 > > If, however, you knew that there are 1 or 2 fields only that you want > > to make sure are not modified concurrently, ASSERT_EXCLUSIVE_WRITER + > > data_race() would probably work well (or even ASSERT_EXCLUSIVE_ACCESS > > if you want to make sure there are no writers nor _readers_). >=20 > I am testing an idea that just do, >=20 > lockdep_assert_held_write(&orig->vm_mm->mmap_sem); > *new =3D data_race(*orig); >=20 > The idea is that as long as we have the exclusive mmap_sem held in all = paths > (auditing indicated so), no writer should be able to mess up our vm_are= a_struct > except the "shared.rb" field which has no harm. Well, some fields protected by page_table_lock and can be written to without exclusive mmap_sem. Probably even without any mmap_sem: pin mm_struct + page_table_lock should be enough. --=20 Kirill A. Shutemov