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=-14.4 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, 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 0EAC1C4727E for ; Wed, 7 Oct 2020 14:43:26 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 66C7B212CC for ; Wed, 7 Oct 2020 14:43:25 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="nkOVjFaa" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 66C7B212CC 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 B9690900002; Wed, 7 Oct 2020 10:43:24 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B44898E0001; Wed, 7 Oct 2020 10:43:24 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9E4AC900002; Wed, 7 Oct 2020 10:43:24 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0018.hostedemail.com [216.40.44.18]) by kanga.kvack.org (Postfix) with ESMTP id 6271E8E0001 for ; Wed, 7 Oct 2020 10:43:24 -0400 (EDT) Received: from smtpin08.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 046E5180AD80F for ; Wed, 7 Oct 2020 14:43:24 +0000 (UTC) X-FDA: 77345397528.08.loss09_5c17ba9271d0 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin08.hostedemail.com (Postfix) with ESMTP id DBDC71819E769 for ; Wed, 7 Oct 2020 14:43:23 +0000 (UTC) X-HE-Tag: loss09_5c17ba9271d0 X-Filterd-Recvd-Size: 5284 Received: from mail-ej1-f65.google.com (mail-ej1-f65.google.com [209.85.218.65]) by imf09.hostedemail.com (Postfix) with ESMTP for ; Wed, 7 Oct 2020 14:43:23 +0000 (UTC) Received: by mail-ej1-f65.google.com with SMTP id md26so3307718ejb.10 for ; Wed, 07 Oct 2020 07:43:23 -0700 (PDT) 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; bh=yaN1rjyLrPsyFHJUcCMRII0CnG4UdUmiNrPeZE3+a2o=; b=nkOVjFaap1/yZl4CUDg4K4Ox8KSB9wy9sJrCPdFITMcuwfROhxDXnBr9RlyK9JOwM1 VEAi9SVibNPsv71oy6nhtbqsusqW+/QiwUIyumUUh6zDmPusa/d38voD+N5QCxtT+5f/ nCjBDNyPjCzVz7ySiHW+pDlThvHFopXt0tucDzNe3e51j5RFzbT8e7VerZ/62jYKjCVh y0cBmyibSPFBSvmMxoM2VWww8R5ehMlVAvmJbE7YH+sLVtXF2ZcP8ulxgyDu8nOH9oZW KiircQAwFxmSIXUj/e/0IYDv/uPIpHIPSX+blkBd56Qpag2B3btDwunrD7oqikY7az1J /dsA== 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; bh=yaN1rjyLrPsyFHJUcCMRII0CnG4UdUmiNrPeZE3+a2o=; b=QXI9m/Baa3w3Y3r5bUiDQuYWein5R/1FBggak806sUxy/lxE+KODwRwUB+O9mLnRVQ K2GyB7d88MTUUyt5o7TkQrJtOqlWbtw+pLvFLDJ6SWT9euPI/nhtz4lIJGLpdbdiXATV sWbPoKppLXARg1Dj7jxdSlMeJ3Ofo4KBJnzdkZDKpqSC64w+yP3S3jpTTpiuZCGp4XmZ uzv6WviTLUQetF6nDvFYnuIgO+D4je7NHEcn46Hl4bfWKqzXZP1QBf5AUCohbcTvWHWV Zd+bGVj0JsapbZivN49IwiczZLviFfsUGHSjVApt0y7zF9LmaINT8c8pVI0LYjod0cn7 C1Eg== X-Gm-Message-State: AOAM530h8LIFXMIj9itfku1v0sPgCEHLI8KyRF5p/uv6UeXARxb6jaPt OhFHwjmX/d//66xZDl6FoKX3TNfDrUeE50OzkXa5qQ== X-Google-Smtp-Source: ABdhPJzcK0C/z7p/R/rO2dkoBnGMVzKrffZV6M0+dNfgCx+3Vi4jT221WFvbiB0KTziaharLk7wRJUaJVTHbRe80EQQ= X-Received: by 2002:a17:906:33c8:: with SMTP id w8mr3659464eja.233.1602081801820; Wed, 07 Oct 2020 07:43:21 -0700 (PDT) MIME-Version: 1.0 References: <20201007073932.865218-1-jannh@google.com> <20201007123544.GA11433@infradead.org> In-Reply-To: <20201007123544.GA11433@infradead.org> From: Jann Horn Date: Wed, 7 Oct 2020 16:42:55 +0200 Message-ID: Subject: Re: [PATCH 1/2] mm/mprotect: Call arch_validate_prot under mmap_lock and with length To: Christoph Hellwig , Michael Ellerman , Benjamin Herrenschmidt , Paul Mackerras , linuxppc-dev@lists.ozlabs.org Cc: "David S. Miller" , sparclinux@vger.kernel.org, Andrew Morton , Linux-MM , kernel list , Khalid Aziz , Anthony Yznaga , Catalin Marinas , Will Deacon , Linux ARM , Dave Kleikamp Content-Type: text/plain; charset="UTF-8" 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, Oct 7, 2020 at 2:35 PM Christoph Hellwig wrote: > On Wed, Oct 07, 2020 at 09:39:31AM +0200, Jann Horn wrote: > > diff --git a/arch/powerpc/kernel/syscalls.c b/arch/powerpc/kernel/syscalls.c > > index 078608ec2e92..b1fabb97d138 100644 > > --- a/arch/powerpc/kernel/syscalls.c > > +++ b/arch/powerpc/kernel/syscalls.c > > @@ -43,7 +43,7 @@ static inline long do_mmap2(unsigned long addr, size_t len, > > { > > long ret = -EINVAL; > > > > - if (!arch_validate_prot(prot, addr)) > > + if (!arch_validate_prot(prot, addr, len)) > > This call isn't under mmap lock. I also find it rather weird as the > generic code only calls arch_validate_prot from mprotect, only powerpc > also calls it from mmap. > > This seems to go back to commit ef3d3246a0d0 > ("powerpc/mm: Add Strong Access Ordering support") I'm _guessing_ the idea in the generic case might be that mmap() doesn't check unknown bits in the protection flags, and therefore maybe people wanted to avoid adding new error cases that could be caused by random high bits being set? So while the mprotect() case checks the flags and refuses unknown values, the mmap() code just lets the architecture figure out which bits are actually valid to set (via arch_calc_vm_prot_bits()) and silently ignores the rest? And powerpc apparently decided that they do want to error out on bogus prot values passed to their version of mmap(), and in exchange, assume in arch_calc_vm_prot_bits() that the protection bits are valid? powerpc's arch_validate_prot() doesn't actually need the mmap lock, so I think this is fine-ish for now (as in, while the code is a bit unclean, I don't think I'm making it worse, and I don't think it's actually buggy). In theory, we could move the arch_validate_prot() call over into the mmap guts, where we're holding the lock, and gate it on the architecture or on some feature CONFIG that powerpc can activate in its Kconfig. But I'm not sure whether that'd be helping or making things worse, so when I sent this patch, I deliberately left the powerpc stuff as-is.