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=-8.4 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL 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 50EACC3403B for ; Tue, 18 Feb 2020 14:09:38 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 152AA2176D for ; Tue, 18 Feb 2020 14:09:38 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="ZnoNvW0J" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 152AA2176D Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id B430C6B0007; Tue, 18 Feb 2020 09:09:37 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id AF2E26B0008; Tue, 18 Feb 2020 09:09:37 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A2FCC6B000A; Tue, 18 Feb 2020 09:09:37 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0177.hostedemail.com [216.40.44.177]) by kanga.kvack.org (Postfix) with ESMTP id 8C7986B0007 for ; Tue, 18 Feb 2020 09:09:37 -0500 (EST) Received: from smtpin08.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 3BDB8181AEF1D for ; Tue, 18 Feb 2020 14:09:37 +0000 (UTC) X-FDA: 76503430794.08.steam65_8e1ed6f291945 X-HE-Tag: steam65_8e1ed6f291945 X-Filterd-Recvd-Size: 5669 Received: from mail-ot1-f68.google.com (mail-ot1-f68.google.com [209.85.210.68]) by imf23.hostedemail.com (Postfix) with ESMTP for ; Tue, 18 Feb 2020 14:09:36 +0000 (UTC) Received: by mail-ot1-f68.google.com with SMTP id j20so19623786otq.3 for ; Tue, 18 Feb 2020 06:09:36 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=Y9w13/tfZTffVdCMe2kCDHSUE9opbbsLGQSUEHoko5U=; b=ZnoNvW0JGNymhsudEZqj4eoZL6SliYBlFQN7OmbbDWtZ+DW/BAxE3hwh/GxzzOUq4I M14C03Y2T+WaaeCiZoUFUPZV9q4zgIZs8yBv8AnDW72hrDDM6y0NkeVo7zaJ40jqROow v8qPHUVDlCW+bds4jvDZv36VciX3KYf4LtCp5k4pcr8ly/3m77mKsJuXokfH7/UPJzfH nqTxQusEkwdKvIDkVZC81F8FbvGRU0IT2MTpGKQeUj5F2pk+dSN22zPstNJZzEVW1KAn JXjtLv6fd60NLxAXjxGyN9XhcueGuh4dWszfAnh55nBzaJl+pOp7AYmVDGNaANbVFIyL Aqjg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=Y9w13/tfZTffVdCMe2kCDHSUE9opbbsLGQSUEHoko5U=; b=OYk1QO1hFBZ2dcxLuACwgVjgEsU9tmzbA/deJGH7fGPDx4YSJQrKy7qBpuolurWx7M d6FLUzBebC71ukZBhDvIZ8krDu2Ze8RODXw6Fi+EJ+Mq5rCLceGfHdkUD1VeII9870br rAYSGBWgPIj+2sTRJ4vpZ50MUbkHdNnC1JTGnNAwv6AZSBoThtI2zNJE8qTAiyt2wI/k nlLGyHxnUcEjDZzop0fdlb+risEBxzi3kiz3/4DEckviYYMivmKHpa83Yd55QK5adW3B v0LSVnedqyFwtit7/kdDa4rGIMygd973YFxFYdIcrpEzyzECsIm1c1N8cCDRHPNhH1vR A+ag== X-Gm-Message-State: APjAAAUPiPrXVLG1Bw5kZADffFhdQc6pxOzkQ0apnf2hHeplwGRblNxh Z2a0mnWzmCKyRrjlEJX6p8BGGM+O2Ao3n4BdV4FIyw== X-Google-Smtp-Source: APXvYqxYlKjO1s4KexGAjjp0ZYGelLIWTBxCcyZwR5o5/JwGl/0bZDE9Ceb/vNAMImYSSQ1HPgvU9yI7CPpyQS9x/EM= X-Received: by 2002:a9d:66d1:: with SMTP id t17mr16550705otm.233.1582034975091; Tue, 18 Feb 2020 06:09:35 -0800 (PST) MIME-Version: 1.0 References: <20200218103002.6rtjreyqjepo3yxe@box> <93E6B243-9A0F-410C-8EE4-9D57E28AF5AF@lca.pw> In-Reply-To: <93E6B243-9A0F-410C-8EE4-9D57E28AF5AF@lca.pw> From: Marco Elver Date: Tue, 18 Feb 2020 15:09:22 +0100 Message-ID: Subject: Re: [PATCH -next] fork: annotate a data race in vm_area_dup() To: Qian Cai Cc: "Kirill A. Shutemov" , Andrew Morton , Linux Memory Management List , LKML , Peter Zijlstra , syzbot , syzkaller-bugs Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bogosity: Ham, tests=bogofilter, spamicity=0.000002, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, 18 Feb 2020 at 13:40, Qian Cai wrote: > > > > > On Feb 18, 2020, at 5:29 AM, Kirill A. Shutemov = wrote: > > > > I think I've got this: > > > > 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 protec= ted > > by i_mmap_lock. But this is fine because the read value will be > > overwritten on the following __vma_link_file() under proper protectecti= on. > > Right, multiple processes could share the same file-based address space w= here those vma have been linked into address_space::i_mmap via vm_area_stru= ct::shared.rb. Thus, the reader could see its shared.rb linkage pointers go= t updated by other processes. > > > > > So the fix is correct, but justificaiton is lacking. > > > > Also, I would like to more fine-grained annotation: marking with > > data_race() 200 bytes copy may hide other issues. > > That is the harder part where I don=E2=80=99t think we have anything for = that today. Macro, any suggestions? ASSERT_IGNORE_FIELD()? There is no nice interface I can think of. All options will just cause more problems, inconsistencies, or annoyances. 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". This option doesn't quite work, unless you just restrict it to 1 field (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? If you wanted something similar to ASSERT_EXCLUSIVE_BITS, it'd have to 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): 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); 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. 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_). Thanks, -- Marco