xenconsole: adjust pty opening error checking and handling
authorMatthew Daley <mattd@bugfuzz.com>
Sat, 14 Dec 2013 01:04:47 +0000 (14:04 +1300)
committerIan Jackson <Ian.Jackson@eu.citrix.com>
Mon, 16 Dec 2013 11:55:22 +0000 (11:55 +0000)
Currently we check the pty path received from xenstore with access(); if
it indicates that the pty is not accessible, we loop around and wait for
a new path to appear in xenstore.

This has several issues:
* If a path has been written to xenstore, it can be assumed that that
  pty should already be accessible to xenconsole, and hence any error
  that occurs while trying to open it should be fatal and not ignored
* If access() indicates no access to the pty, the memory allocated for
  the path is leaked when going around the loop again
* The accessibility of the pty could change between the access() and
  open() calls, leading to a TOCTOU race (this is what Coverity is
  complaining about).

By removing the explicit access() check and just erroring out whenever
open() fails, we fix all these issues.

Coverity-ID: 1056047
Signed-off-by: Matthew Daley <mattd@bugfuzz.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
tools/console/client/main.c

index 38c856a11261519022ec5cc1017bd6700c7cff1a..324200890d83e2b84d7565acd922917866f9bf91 100644 (file)
@@ -116,12 +116,9 @@ static int get_pty_fd(struct xs_handle *xs, char *path, int seconds)
                         * disambiguate: just read the pty path */
                        pty_path = xs_read(xs, XBT_NULL, path, &len);
                        if (pty_path != NULL) {
-                               if (access(pty_path, R_OK|W_OK) != 0)
-                                       continue;
                                pty_fd = open(pty_path, O_RDWR | O_NOCTTY);
-                               if (pty_fd == -1) 
-                                       err(errno, "Could not open tty `%s'", 
-                                           pty_path);
+                               if (pty_fd == -1)
+                                       err(errno, "Could not open tty `%s'", pty_path);
                                free(pty_path);
                        }
                }