Re: [PATCH] Fix getlogin() to check only stdin to get a valid tty

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