xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Matthew Daley <mattd@bugfuzz.com>
To: xen-devel@lists.xen.org
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
	Matthew Daley <mattd@bugfuzz.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	Ian Campbell <ian.campbell@citrix.com>,
	Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Subject: [PATCH v3] xenconsole: adjust pty opening error checking and handling
Date: Sat, 14 Dec 2013 14:04:47 +1300	[thread overview]
Message-ID: <1386983087-4777-1-git-send-email-mattd@bugfuzz.com> (raw)
In-Reply-To: <21163.15704.53104.985508@mariner.uk.xensource.com>

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>
---
v3: Improve commit message to explain what behaviours are fixed (and
hence changed)

 tools/console/client/main.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/tools/console/client/main.c b/tools/console/client/main.c
index 38c856a..3242008 100644
--- a/tools/console/client/main.c
+++ b/tools/console/client/main.c
@@ -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);
 			}
 		}
-- 
1.7.10.4

  parent reply	other threads:[~2013-12-14  1:04 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-30  3:42 [PATCH] xenconsole: merge pty access check into when it is opened Matthew Daley
2013-12-01 11:41 ` Andrew Cooper
2013-12-01 23:14   ` Matthew Daley
2013-12-01 23:29     ` Andrew Cooper
2013-12-02  2:41       ` [PATCH v2] " Matthew Daley
2013-12-02 10:30         ` Andrew Cooper
2013-12-13  5:59         ` Matthew Daley
2013-12-02 11:51       ` [PATCH] " Ian Jackson
2013-12-02 11:53         ` Andrew Cooper
2013-12-13 17:01     ` [PATCH] xenconsole: merge pty access check into when it is opened [and 1 more messages] Ian Jackson
2013-12-13 22:54       ` Matthew Daley
2013-12-14  1:04       ` Matthew Daley [this message]
2013-12-16 11:58         ` [PATCH v3] xenconsole: adjust pty opening error checking and handling Ian Jackson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1386983087-4777-1-git-send-email-mattd@bugfuzz.com \
    --to=mattd@bugfuzz.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=ian.campbell@citrix.com \
    --cc=ian.jackson@eu.citrix.com \
    --cc=stefano.stabellini@eu.citrix.com \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).