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=-7.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_ADSP_CUSTOM_MED,DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 83E19C4363D for ; Wed, 7 Oct 2020 14:44:41 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id F1320208C7 for ; Wed, 7 Oct 2020 14:44:40 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="gT4vICHq"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=google.com header.i=@google.com header.b="nkOVjFaa" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org F1320208C7 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:To:Subject:Message-ID:Date:From:In-Reply-To: References:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=0+md1YZgjvm17+Tog4wUT64eWVJs5mmtllULZ2+gq38=; b=gT4vICHqTxagBrk4wxX8rcCTo I8L/elFqWd8mvw9VOmHvAGPLtvEwLb0hq2pPU+KWzbtZSs+AdL8a4xe8vKIouevlxozGzXz6FRXtP m2G6Yb66E2Spqs6kR8SkXE9TxGW95UNuhBuGUgmgA/TwSYkQ83ySVQF7YdOsWT5dG2LUiUVeUAFyo QGk7rnHVyaA4epqT7/Xj47d67smD26YS6XmmVqxnUKJu+2EZxM7uEtnVd3ZqD67sgEvnv6dmNuJ5J OeqqiW1Ag98kFZiVbhmLSjrfloOFRK9r66uas2Ld7bnPoLOu1qRpewsIOWrg+4GWIiFjZxQFGGY0b SG/mCGPFw==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kQAf9-00073J-44; Wed, 07 Oct 2020 14:43:27 +0000 Received: from mail-ej1-x642.google.com ([2a00:1450:4864:20::642]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kQAf5-00071L-KQ for linux-arm-kernel@lists.infradead.org; Wed, 07 Oct 2020 14:43:24 +0000 Received: by mail-ej1-x642.google.com with SMTP id ce10so3331629ejc.5 for ; Wed, 07 Oct 2020 07:43:22 -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=npPFUt7cOG01CpVgj8+XWTgAEJtsVtZlKKg+tHMPxzQLYjsPe25iiwI857XiN4ur8N 26OK49PUFAVLirdsiHjlFe+CcS9/YGHMyuodF7Rj15jZGgKCQs0UwK2kChYH2oOWO8/8 vyHhBql+FNw2vglWo68P9a8oBaXoDM0u5l4UDw6Pp2hGdG5rF8sjrWSSFhsJ9puyFkk8 5aUcjA9yvWSYhZ87CiqO+eKrmr+1VrfZG8EDCiwLjYGT8+rkmmnZ76MwIk1VIyLOv71Y p0dZvfLRmTLIxjMA2YyFDxgmnRPPlUbCRHTKhDK61XK2SkAZxKvk5Prgxh/RpH/iVgyA cMgA== X-Gm-Message-State: AOAM530klbP9zYWr913BPrC3pwip2ccxPwAlW+uDI4C1dyIiSA9p91ql t9uIYzYhWENEqcgZufg+08cFMc3Eda1r0sCu9JZbig== 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 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201007_104323_734876_90C91C3E X-CRM114-Status: GOOD ( 22.98 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Dave Kleikamp , Catalin Marinas , kernel list , Linux-MM , Khalid Aziz , sparclinux@vger.kernel.org, Anthony Yznaga , Andrew Morton , Will Deacon , "David S. Miller" , Linux ARM Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org 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. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel