From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 65353 invoked by alias); 26 Jul 2015 20:48:31 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 65324 invoked by uid 89); 26 Jul 2015 20:48:30 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL,BAYES_00,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-yk0-f171.google.com Received: from mail-yk0-f171.google.com (HELO mail-yk0-f171.google.com) (209.85.160.171) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Sun, 26 Jul 2015 20:48:29 +0000 Received: by ykax123 with SMTP id x123so55531322yka.1 for ; Sun, 26 Jul 2015 13:48:27 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.13.237.1 with SMTP id w1mr26632760ywe.132.1437943707154; Sun, 26 Jul 2015 13:48:27 -0700 (PDT) Received: by 10.13.233.198 with HTTP; Sun, 26 Jul 2015 13:48:26 -0700 (PDT) In-Reply-To: <874mkq4t58.fsf@redhat.com> References: <1437761993-18758-1-git-send-email-sergiodj@redhat.com> <1437869674-7880-1-git-send-email-sergiodj@redhat.com> <874mkq4t58.fsf@redhat.com> Date: Sun, 26 Jul 2015 20:48:00 -0000 Message-ID: Subject: Re: [PATCH v3] Make sure GDB uses a valid shell when starting the inferior and to perform the "shell" command From: Doug Evans To: Sergio Durigan Junior Cc: GDB Patches , Eli Zaretskii Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2015-07/txt/msg00780.txt.bz2 On Sun, Jul 26, 2015 at 12:26 PM, Sergio Durigan Junior wrote: > On Sunday, July 26 2015, Doug Evans wrote: >> ... >> Hi. >> >> I'd like to not have this patch checked in, at least not yet. >> >> I'm going to leave security as a separate thread. >> The topic here is just convenience and assistance (IIUC - >> please correct me if I'm wrong). > > It is just assistance, indeed.. Security is definitely not the focus > here. > >> Having an internally hardcoded list of shells (good or bad) suggests >> to me there's got to be a better way. > > I'm definitely open to suggestions. > >> Another thing that bothers me is that if SHELL >> is set to something gdb thinks is bad, gdb will >> try to be "clever" and override that setting. >> If a tool is going to be helpful, I think it >> also needs a mode to not be. It's hard to >> work around hardwired cleverness when >> you don't want it. Hopefully in this instance >> we can avoid adding an option though. > > Yeah. This can be easily fixed with (yet another) setting. 'set > use-valid-shell on/off', maybe? > >> As a strawman, what if gdb first tests $SHELL >> (e.g., $SHELL -c 'exit 42' or some such) >> and if that doesn't work warn the user, >> but otherwise leave things as is? >> One could defer doing the test until the first >> need for $SHELL. >> And if $SHELL isn't usable, leave it to the >> user to fix the problem. > > So you're suggesting that we only warn the user about the invalid shell, > instead of deciding to use /bin/sh without asking her? > > As much as I think it *is* useful to have GDB default to /bin/sh if > $SHELL is /sbin/nologin (for example), I am OK with just warning the > user without taking any action. > > So, to summarize: what would you think of a patch that: > > - tested $SHELL (as you proposed; $SHELL -c 'exit 42'). > > - if the test fails, warn the user about it. If 'set use-valid-shell' > is on, continue using /bin/sh; otherwise, just error out. > > ? > > Thanks, Assuming others are ok with it, I'd say let's go with the test, and leave use-valid-shell for another day. IIUC we tripped over this because of a misconfigured build-bot, which we can easily fix. It's not clear to me that a new user option is warranted. They're using gdb. If they don't know about $SHELL and /bin/sh we can educate them - and one place we can do that is in the warning we print if the test fails. [I'm all for having more descriptive/explanatory warnings/errors that assist users in fixing the issue.]