From patchwork Thu Jul 27 14:54:31 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thiago Jung Bauermann X-Patchwork-Id: 794467 Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3xJFTT59QJz9s5L for ; Fri, 28 Jul 2017 00:58:05 +1000 (AEST) Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 3xJFTT4BYdzDrLV for ; Fri, 28 Jul 2017 00:58:05 +1000 (AEST) X-Original-To: linuxppc-dev@lists.ozlabs.org Delivered-To: linuxppc-dev@lists.ozlabs.org Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 3xJFRk2yl5zDrH2 for ; Fri, 28 Jul 2017 00:56:34 +1000 (AEST) Received: from pps.filterd (m0098404.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id v6REu2Bi074516 for ; Thu, 27 Jul 2017 10:56:32 -0400 Received: from e24smtp05.br.ibm.com (e24smtp05.br.ibm.com [32.104.18.26]) by mx0a-001b2d01.pphosted.com with ESMTP id 2byfha0km2-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Thu, 27 Jul 2017 10:56:23 -0400 Received: from localhost by e24smtp05.br.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 27 Jul 2017 11:54:45 -0300 Received: from d24relay03.br.ibm.com (9.13.39.225) by e24smtp05.br.ibm.com (10.172.0.141) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Thu, 27 Jul 2017 11:54:43 -0300 Received: from d24av04.br.ibm.com (d24av04.br.ibm.com [9.8.31.97]) by d24relay03.br.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id v6REsght38273192 for ; Thu, 27 Jul 2017 11:54:42 -0300 Received: from d24av04.br.ibm.com (localhost [127.0.0.1]) by d24av04.br.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id v6REsgAA006448 for ; Thu, 27 Jul 2017 11:54:43 -0300 Received: from morokweng ([9.80.210.138]) by d24av04.br.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id v6REsXFY006372 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Thu, 27 Jul 2017 11:54:36 -0300 References: <1500177424-13695-1-git-send-email-linuxram@us.ibm.com> <1500177424-13695-20-git-send-email-linuxram@us.ibm.com> From: Thiago Jung Bauermann To: Ram Pai Subject: Re: [RFC v6 19/62] powerpc: ability to create execute-disabled pkeys In-reply-to: <1500177424-13695-20-git-send-email-linuxram@us.ibm.com> Date: Thu, 27 Jul 2017 11:54:31 -0300 MIME-Version: 1.0 X-TM-AS-MML: disable x-cbid: 17072714-0032-0000-0000-00000576851C X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17072714-0033-0000-0000-000011FCE316 Message-Id: <87bmo63p7c.fsf@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2017-07-27_08:, , signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=1 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1706020000 definitions=main-1707270234 X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-arch@vger.kernel.org, corbet@lwn.net, arnd@arndb.de, linux-doc@vger.kernel.org, x86@kernel.org, dave.hansen@intel.com, linux-kernel@vger.kernel.org, mhocko@kernel.org, linux-mm@kvack.org, mingo@redhat.com, paulus@samba.org, aneesh.kumar@linux.vnet.ibm.com, linux-kselftest@vger.kernel.org, akpm@linux-foundation.org, linuxppc-dev@lists.ozlabs.org, khandual@linux.vnet.ibm.com Errors-To: linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org Sender: "Linuxppc-dev" Ram Pai writes: > --- a/arch/powerpc/include/asm/pkeys.h > +++ b/arch/powerpc/include/asm/pkeys.h > @@ -2,6 +2,18 @@ > #define _ASM_PPC64_PKEYS_H > > extern bool pkey_inited; > +/* override any generic PKEY Permission defines */ > +#undef PKEY_DISABLE_ACCESS > +#define PKEY_DISABLE_ACCESS 0x1 > +#undef PKEY_DISABLE_WRITE > +#define PKEY_DISABLE_WRITE 0x2 > +#undef PKEY_DISABLE_EXECUTE > +#define PKEY_DISABLE_EXECUTE 0x4 > +#undef PKEY_ACCESS_MASK > +#define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS |\ > + PKEY_DISABLE_WRITE |\ > + PKEY_DISABLE_EXECUTE) > + Is it ok to #undef macros from another header? Especially since said header is in uapi (include/uapi/asm-generic/mman-common.h). Also, it's unnecessary to undef the _ACCESS and _WRITE macros since they are identical to the original definition. And since these macros are originally defined in an uapi header, the powerpc-specific ones should be in an uapi header as well, if I understand it correctly. An alternative solution is to define only PKEY_DISABLE_EXECUTE in arch/powerpc/include/uapi/asm/mman.h and then test for its existence to properly define PKEY_ACCESS_MASK in include/uapi/asm-generic/mman-common.h. What do you think of the code below? > diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c > index 98d0391..b9ad98d 100644 > --- a/arch/powerpc/mm/pkeys.c > +++ b/arch/powerpc/mm/pkeys.c > @@ -73,6 +73,7 @@ int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey, > unsigned long init_val) > { > u64 new_amr_bits = 0x0ul; > + u64 new_iamr_bits = 0x0ul; > > if (!is_pkey_enabled(pkey)) > return -1; > @@ -85,5 +86,14 @@ int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey, > > init_amr(pkey, new_amr_bits); > > + /* > + * By default execute is disabled. > + * To enable execute, PKEY_ENABLE_EXECUTE > + * needs to be specified. > + */ > + if ((init_val & PKEY_DISABLE_EXECUTE)) > + new_iamr_bits |= IAMR_EX_BIT; > + > + init_iamr(pkey, new_iamr_bits); > return 0; > } The comment seems to be from an earlier version which has the logic inverted, and there is no PKEY_ENABLE_EXECUTE. Should the comment be updated to the following? By default execute is enabled. To disable execute, PKEY_DISABLE_EXECUTE needs to be specified. diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h index e31f5ee8e81f..67e6a3a343ae 100644 --- a/arch/powerpc/include/asm/pkeys.h +++ b/arch/powerpc/include/asm/pkeys.h @@ -4,17 +4,6 @@ #include extern bool pkey_inited; -/* override any generic PKEY Permission defines */ -#undef PKEY_DISABLE_ACCESS -#define PKEY_DISABLE_ACCESS 0x1 -#undef PKEY_DISABLE_WRITE -#define PKEY_DISABLE_WRITE 0x2 -#undef PKEY_DISABLE_EXECUTE -#define PKEY_DISABLE_EXECUTE 0x4 -#undef PKEY_ACCESS_MASK -#define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS |\ - PKEY_DISABLE_WRITE |\ - PKEY_DISABLE_EXECUTE) #define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | \ VM_PKEY_BIT3 | VM_PKEY_BIT4) diff --git a/arch/powerpc/include/uapi/asm/mman.h b/arch/powerpc/include/uapi/asm/mman.h index ab45cc2f3101..dee43feb7c53 100644 --- a/arch/powerpc/include/uapi/asm/mman.h +++ b/arch/powerpc/include/uapi/asm/mman.h @@ -45,4 +45,6 @@ #define MAP_HUGE_1GB (30 << MAP_HUGE_SHIFT) /* 1GB HugeTLB Page */ #define MAP_HUGE_16GB (34 << MAP_HUGE_SHIFT) /* 16GB HugeTLB Page */ +#define PKEY_DISABLE_EXECUTE 0x4 + #endif /* _UAPI_ASM_POWERPC_MMAN_H */ diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c index 72eb9a1bde79..777f8f8dff47 100644 --- a/arch/powerpc/mm/pkeys.c +++ b/arch/powerpc/mm/pkeys.c @@ -12,7 +12,7 @@ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for * more details. */ -#include +#include #include /* PKEY_* */ bool pkey_inited; diff --git a/include/uapi/asm-generic/mman-common.h b/include/uapi/asm-generic/mman-common.h index 8c27db0c5c08..93e3841d9ada 100644 --- a/include/uapi/asm-generic/mman-common.h +++ b/include/uapi/asm-generic/mman-common.h @@ -74,7 +74,15 @@ #define PKEY_DISABLE_ACCESS 0x1 #define PKEY_DISABLE_WRITE 0x2 + +/* The arch-specific code may define PKEY_DISABLE_EXECUTE */ +#ifdef PKEY_DISABLE_EXECUTE +#define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS | \ + PKEY_DISABLE_WRITE | \ + PKEY_DISABLE_EXECUTE) +#else #define PKEY_ACCESS_MASK (PKEY_DISABLE_ACCESS |\ PKEY_DISABLE_WRITE) +#endif #endif /* __ASM_GENERIC_MMAN_COMMON_H */