From patchwork Tue Jun 27 07:17:57 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom de Vries X-Patchwork-Id: 781017 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3wxcj72qQJz9s5L for ; Tue, 27 Jun 2017 17:18:35 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="m7Ug7190"; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:cc:references:from:message-id:date:mime-version :in-reply-to:content-type; q=dns; s=default; b=iu43cxTq968/LCulC dQiIYH9kUkgsCKtpzIrqXEj0CUuBIeHvF9kKfUOYjrRqbMLgZaTuW1tOF8QxS79J Lj4KDelYflUI+CUQCzniuDzXFFG67q5IQpq9WarP186hCsK7v/cpe8tNQGmG73fA q0SCM+YySewbp3Ggbt7mU52pMM= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:cc:references:from:message-id:date:mime-version :in-reply-to:content-type; s=default; bh=yehVeqggpq+9Qnw9Ag+lVbF aUos=; b=m7Ug7190QLsu5ci0XzadEiTq2LdhU2jV1fBK7m160FagaCs4aOEkRww Axv/nGSQrkSHZZtlUGQws6noLmzMKI9tde9lwvrg214Ka1nnYzRlPwIlS7K0CJ9G jKt6hf4DQrkLA4iCCSTcTqWFFLXFXkD/YpiZB6G3ufFqNUbOnaHQ= Received: (qmail 33839 invoked by alias); 27 Jun 2017 07:18:14 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 33722 invoked by uid 89); 27 Jun 2017 07:18:13 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.5 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS, URIBL_RED autolearn=ham version=3.3.2 spammy= X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 27 Jun 2017 07:18:10 +0000 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=SVR-IES-MBX-04.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1dPklC-0001H4-94 from Tom_deVries@mentor.com ; Tue, 27 Jun 2017 00:18:06 -0700 Received: from [127.0.0.1] (137.202.0.87) by SVR-IES-MBX-04.mgc.mentorg.com (139.181.222.4) with Microsoft SMTP Server (TLS) id 15.0.1263.5; Tue, 27 Jun 2017 08:18:01 +0100 Subject: [PATCH] Use secure_getenv for GOMP_DEBUG To: Jakub Jelinek CC: Joseph Myers , GCC Patches , Thomas Schwinge References: <2413b0f6-9cb2-243f-d805-08323a9c9a0a@mentor.com> <1de94c83-96da-f380-9964-1472f63270c9@mentor.com> <20170626152911.GK2123@tucnak> From: Tom de Vries Message-ID: <9110a6a9-43dc-52ca-aabf-20c73ac73775@mentor.com> Date: Tue, 27 Jun 2017 09:17:57 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: <20170626152911.GK2123@tucnak> X-ClientProxiedBy: svr-ies-mbx-01.mgc.mentorg.com (139.181.222.1) To SVR-IES-MBX-04.mgc.mentorg.com (139.181.222.4) [ was: Re: [PATCH, 2/4] Handle GOMP_OPENACC_NVPTX_{DISASM,SAVE_TEMPS} in libgomp nvptx plugin ] On 06/26/2017 05:29 PM, Jakub Jelinek wrote: > On Mon, Jun 26, 2017 at 03:26:57PM +0000, Joseph Myers wrote: >> On Mon, 26 Jun 2017, Tom de Vries wrote: >> >>>> 2. Handle GOMP_OPENACC_NVPTX_{DISASM,SAVE_TEMPS} in libgomp nvptx plugin >>> >>> This patch adds handling of: >>> - GOMP_OPENACC_NVPTX_SAVE_TEMPS=[01], and >>> - GOMP_OPENACC_NVPTX_DISASM=[01] >>> >>> The filename used for dumping the module is plugin-nvptx..cubin. >> >> Are you sure this use of getenv and writing to that file is safe for >> setuid/setgid programs? I'd expect you to need to use secure_getenv as in >> plugin-hsa.c; certainly for anything that could results in writes to a >> file like that. > > Yeah, definitely it should be using secure_getenv/__secure_getenv. > And IMNSHO GOMP_DEBUG too. > This patch uses secure_getenv for GOMP_DEBUG. It factors out the secure_getenv code from plugin-hsa.c into libgomp/secure_getenv.h, and reuses it in env.c. I've added _GNU_SOURCE before the libgomp.h include in env.c to make sure that secure_getenv (imported from stdlib.h) is available. I've also added a test-case that sets GOMP_DEBUG to 1 and verifies that some output is generated. Build for c-only on x86_64 without accelerator, tested libgomp -m64/-m32. OK if x86_64 bootstrap and reg-test succeeds? Thanks, - Tom Use secure_getenv for GOMP_DEBUG 2017-06-26 Tom de Vries * env.c (parse_unsigned_long_1): Factor out of ... (parse_unsigned_long): ... here. (parse_int_1): Factor out of ... (parse_int): ... here. (parse_int_secure): New function. (initialize_env): Use parse_int_secure for GOMP_DEBUG. * secure_getenv.h: Factor out of ... * plugin/plugin-hsa.c: ... here. * testsuite/libgomp.oacc-c-c++-common/gomp-debug-env.c: New test. --- libgomp/env.c | 44 +++++++++++++++--- libgomp/plugin/plugin-hsa.c | 27 +---------- libgomp/secure_getenv.h | 53 ++++++++++++++++++++++ .../libgomp.oacc-c-c++-common/gomp-debug-env.c | 13 ++++++ 4 files changed, 104 insertions(+), 33 deletions(-) diff --git a/libgomp/env.c b/libgomp/env.c index ced752d..802c73b 100644 --- a/libgomp/env.c +++ b/libgomp/env.c @@ -26,6 +26,7 @@ /* This file defines the OpenMP internal control variables and arranges for them to be initialized from environment variables at startup. */ +#define _GNU_SOURCE #include "libgomp.h" #include "gomp-constants.h" #include @@ -58,6 +59,8 @@ #endif #endif /* LIBGOMP_OFFLOADED_ONLY */ +#include "secure_getenv.h" + struct gomp_task_icv gomp_global_icv = { .nthreads_var = 1, .thread_limit_var = UINT_MAX, @@ -171,15 +174,17 @@ parse_schedule (void) } /* Parse an unsigned long environment variable. Return true if one was - present and it was successfully parsed. */ + present and it was successfully parsed. If SECURE, use secure_getenv to the + environment variable. */ static bool -parse_unsigned_long (const char *name, unsigned long *pvalue, bool allow_zero) +parse_unsigned_long_1 (const char *name, unsigned long *pvalue, bool allow_zero, + bool secure) { char *env, *end; unsigned long value; - env = getenv (name); + env = (secure ? secure_getenv (name) : getenv (name)); if (env == NULL) return false; @@ -206,14 +211,23 @@ parse_unsigned_long (const char *name, unsigned long *pvalue, bool allow_zero) return false; } +/* As parse_unsigned_long_1, but always use getenv. */ + +static bool +parse_unsigned_long (const char *name, unsigned long *pvalue, bool allow_zero) +{ + return parse_unsigned_long_1 (name, pvalue, allow_zero, false); +} + /* Parse a positive int environment variable. Return true if one was - present and it was successfully parsed. */ + present and it was successfully parsed. If SECURE, use secure_getenv to the + environment variable. */ static bool -parse_int (const char *name, int *pvalue, bool allow_zero) +parse_int_1 (const char *name, int *pvalue, bool allow_zero, bool secure) { unsigned long value; - if (!parse_unsigned_long (name, &value, allow_zero)) + if (!parse_unsigned_long_1 (name, &value, allow_zero, secure)) return false; if (value > INT_MAX) { @@ -224,6 +238,22 @@ parse_int (const char *name, int *pvalue, bool allow_zero) return true; } +/* As parse_int_1, but use getenv. */ + +static bool +parse_int (const char *name, int *pvalue, bool allow_zero) +{ + return parse_int_1 (name, pvalue, allow_zero, false); +} + +/* As parse_int_1, but use getenv_secure. */ + +static bool +parse_int_secure (const char *name, int *pvalue, bool allow_zero) +{ + return parse_int_1 (name, pvalue, allow_zero, true); +} + /* Parse an unsigned long list environment variable. Return true if one was present and it was successfully parsed. */ @@ -1207,7 +1237,7 @@ initialize_env (void) gomp_global_icv.thread_limit_var = thread_limit_var > INT_MAX ? UINT_MAX : thread_limit_var; } - parse_int ("GOMP_DEBUG", &gomp_debug_var, true); + parse_int_secure ("GOMP_DEBUG", &gomp_debug_var, true); #ifndef HAVE_SYNC_BUILTINS gomp_mutex_init (&gomp_managed_threads_lock); #endif diff --git a/libgomp/plugin/plugin-hsa.c b/libgomp/plugin/plugin-hsa.c index 90ca247..adb07ac 100644 --- a/libgomp/plugin/plugin-hsa.c +++ b/libgomp/plugin/plugin-hsa.c @@ -39,32 +39,7 @@ #include #include "libgomp-plugin.h" #include "gomp-constants.h" - -/* Secure getenv() which returns NULL if running as SUID/SGID. */ -#ifndef HAVE_SECURE_GETENV -#ifdef HAVE___SECURE_GETENV -#define secure_getenv __secure_getenv -#elif defined (HAVE_UNISTD_H) && defined(HAVE_GETUID) && defined(HAVE_GETEUID) \ - && defined(HAVE_GETGID) && defined(HAVE_GETEGID) - -#include - -/* Implementation of secure_getenv() for targets where it is not provided but - we have at least means to test real and effective IDs. */ - -static char * -secure_getenv (const char *name) -{ - if ((getuid () == geteuid ()) && (getgid () == getegid ())) - return getenv (name); - else - return NULL; -} - -#else -#define secure_getenv getenv -#endif -#endif +#include "secure-getenv.h" /* As an HSA runtime is dlopened, following structure defines function pointers utilized by the HSA plug-in. */ diff --git a/libgomp/secure_getenv.h b/libgomp/secure_getenv.h new file mode 100644 index 0000000..6804e61 --- /dev/null +++ b/libgomp/secure_getenv.h @@ -0,0 +1,53 @@ +/* Copyright (C) 2017 Free Software Foundation, Inc. + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 3, or (at your option) +any later version. + +GCC is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +Under Section 7 of GPL version 3, you are granted additional +permissions described in the GCC Runtime Library Exception, version +3.1, as published by the Free Software Foundation. + +You should have received a copy of the GNU General Public License and +a copy of the GCC Runtime Library Exception along with this program; +see the files COPYING3 and COPYING.RUNTIME respectively. If not, see +. */ + +#ifndef _SECURE_GETENV_H +#define _SECURE_GETENV_H 1 + +/* Secure getenv() which returns NULL if running as SUID/SGID. */ +#ifndef HAVE_SECURE_GETENV +#ifdef HAVE___SECURE_GETENV +#define secure_getenv __secure_getenv +#elif defined (HAVE_UNISTD_H) && defined(HAVE_GETUID) && defined(HAVE_GETEUID) \ + && defined(HAVE_GETGID) && defined(HAVE_GETEGID) + +#include + +/* Implementation of secure_getenv() for targets where it is not provided but + we have at least means to test real and effective IDs. */ + +static char * +secure_getenv (const char *name) +{ + if ((getuid () == geteuid ()) && (getgid () == getegid ())) + return getenv (name); + else + return NULL; +} + +#else +#define secure_getenv getenv +#endif +#endif + +#endif /* _SECURE_GETENV_H. */ diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/gomp-debug-env.c b/libgomp/testsuite/libgomp.oacc-c-c++-common/gomp-debug-env.c new file mode 100644 index 0000000..3fc3503 --- /dev/null +++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/gomp-debug-env.c @@ -0,0 +1,13 @@ +/* { dg-do run } */ +/* { dg-set-target-env-var GOMP_DEBUG "1" } */ + +/* Check that GOMP_DEBUG=1 triggers some output. */ + +int +main (void) +{ +#pragma acc parallel + ; +} + +/* { dg-output "GOACC_parallel_keyed" } */