From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.fibranet.cat (www.fibranet.cat [88.99.13.26]) by sourceware.org (Postfix) with ESMTPS id 87AA13858D32 for ; Thu, 13 Jul 2023 16:57:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 87AA13858D32 Authentication-Results: sourceware.org; dmarc=pass (p=reject dis=none) header.from=fibranet.cat Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=fibranet.cat Received: from [192.168.1.2] (233.red-83-41-32.dynamicip.rima-tde.net [83.41.32.233]) (authenticated bits=0) by mail.fibranet.cat (8.15.2/8.15.2) with ESMTPSA id 36DGvX1F1869836 (version=TLSv1.3 cipher=TLS_AES_128_GCM_SHA256 bits=128 verify=NOT) for ; Thu, 13 Jul 2023 18:57:35 +0200 DKIM-Filter: OpenDKIM Filter v2.11.0 mail.fibranet.cat 36DGvX1F1869836 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fibranet.cat; s=default; t=1689267455; bh=x9M9zStyZuUxf5/kjMaBA/17QwTAj8eVThzeyN9A23Y=; h=Date:Subject:To:References:From:In-Reply-To:From; b=DuCKkqZt3PrXX2yLiPZ7sA/+3Ouvg4DZzhBqzmc0AkqD/+ZfnTcBpO2pe+vRZSXyf XLpnirS1Mjv9LI/FxI7JoiWUeSVq/FbIn5ja3In1JrgnIfffNQOFpzPkl2CB4Ttt1x eeilPxTvvAfHLMj70YoyFcwrTBPmyaSfmRE68TJI= Message-ID: <61101fc8-fc4f-8781-98a4-d84be32c126b@fibranet.cat> Date: Thu, 13 Jul 2023 18:57:32 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 Subject: Re: [PATCH] Fix getlogin() to check only stdin to get a valid tty To: newlib@sourceware.org References: <1de3b3ee-7dd8-db16-6e17-365dbd9fde84@fibranet.cat> <74633613-8420-5ad6-2882-6ad14066f781@foss.st.com> <9ad091f5-4ca9-6410-e990-0cd4f1ece86e@foss.st.com> Content-Language: en-US From: Jordi Sanfeliu In-Reply-To: <9ad091f5-4ca9-6410-e990-0cd4f1ece86e@foss.st.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.6.2 (mail.fibranet.cat [88.99.13.26]); Thu, 13 Jul 2023 18:57:35 +0200 (CEST) X-FibraNet-MailScanner-Information: FibraNet E-Mail Virus Protection Service X-FibraNet-MailScanner-ID: 36DGvX1F1869836 X-FibraNet-MailScanner: Found to be clean X-FibraNet-MailScanner-SpamCheck: X-FibraNet-MailScanner-From: jordi@fibranet.cat X-FibraNet-MailScanner-Watermark: 1689872256.4022@E0+Z9CKi/EQvgOdgGIBbxQ X-Spam-Status: No, score=-9.3 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hello, So we have two possible patches to solve this situation: My first original patch that only checks if stdin is a tty and seems to match with glibc comments provided by Torbjörn. The only problem is that it won't work in the scenario when someone pipes some data on stdin to the application: $ echo | ./test $ diff --git a/newlib/libc/unix/getlogin.c b/newlib/libc/unix/getlogin.c index da4f47a95..e646bcb08 100644 --- a/newlib/libc/unix/getlogin.c +++ b/newlib/libc/unix/getlogin.c @@ -16,9 +16,7 @@ getlogin () extern char *ttyname (); char *tty; - if (((tty = ttyname (0)) == 0) - || ((tty = ttyname (1)) == 0) - || ((tty = ttyname (2)) == 0)) + if ((tty = ttyname (0)) == 0) return 0; if ((utmp_fd = open (UTMP_FILE, O_RDONLY)) == -1) And a patch based on the comments of Torbjörn that checks all three fds before returning NULL: diff --git a/newlib/libc/unix/getlogin.c b/newlib/libc/unix/getlogin.c index da4f47a95..5a3f172e7 100644 --- a/newlib/libc/unix/getlogin.c +++ b/newlib/libc/unix/getlogin.c @@ -16,10 +16,10 @@ getlogin () extern char *ttyname (); char *tty; - if (((tty = ttyname (0)) == 0) - || ((tty = ttyname (1)) == 0) - || ((tty = ttyname (2)) == 0)) - return 0; + if ((tty = ttyname (0)) == 0) + if ((tty = ttyname (1)) == 0) + if ((tty = ttyname (2)) == 0) + return 0; if ((utmp_fd = open (UTMP_FILE, O_RDONLY)) == -1) return 0; Any thoughts? Thanks. On 7/13/23 18:25, Torbjorn SVENSSON wrote: > I took a sneak peak at how glibc does this and there is this comment: > >   /* Get name of tty connected to fd 0.  Return NULL if not a tty or >      if fd 0 isn't open.  Note that a lot of documentation says that >      getlogin() is based on the controlling terminal---what they >      really mean is "the terminal connected to standard input".  The >      getlogin() implementation of DEC Unix, SunOS, Solaris, HP-UX all >      return NULL if fd 0 has been closed, so this is the compatible >      thing to do.  Note that ttyname(open("/dev/tty")) on those >      systems returns /dev/tty, so that is not a possible solution for >      getlogin().  */ > > Based on this comment, I guess it would be sane to drop the check on > stdout and stderr, but it would have the consequence that you are not > able to pipe some data on stdin to the application that calls getlogin > as it would fail in that scenario. > > I'm not a maintainer of newlib so I don't really have anything to say > about what path you decide to go. -- Jordi Sanfeliu FIBRANET Network Services Provider https://www.fibranet.cat