xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Matthew Daley <mattd@bugfuzz.com>, xen-devel@lists.xen.org
Cc: Ian Jackson <ian.jackson@eu.citrix.com>,
	Ian Campbell <ian.campbell@citrix.com>,
	Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Subject: Re: [PATCH] xenconsole: merge pty access check into when it is opened
Date: Sun, 1 Dec 2013 11:41:59 +0000	[thread overview]
Message-ID: <529B2086.1080804@citrix.com> (raw)
In-Reply-To: <1385782972-22793-1-git-send-email-mattd@bugfuzz.com>

On 30/11/2013 03:42, Matthew Daley wrote:
> This stops pty_path from being leaked, and removes the toctou race,
> FWIW.
>
> Not sure why it's a separate check to begin with...
>
> Coverity-ID: 1056047
> Signed-off-by: Matthew Daley <mattd@bugfuzz.com>
> ---
>  tools/console/client/main.c |   10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/tools/console/client/main.c b/tools/console/client/main.c
> index 38c856a..c32d3eb 100644
> --- a/tools/console/client/main.c
> +++ b/tools/console/client/main.c
> @@ -116,12 +116,12 @@ 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) {
> +					if (errno != EACCES)
> +						err(errno, "Could not open tty `%s'",
> +							pty_path);

access() can fail for many more reasons than just EACCES.  I would skip
the errno check entirely and always print the error.

~Andrew

> +				}
>  				free(pty_path);
>  			}
>  		}

  reply	other threads:[~2013-12-01 11:41 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 [this message]
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       ` [PATCH v3] xenconsole: adjust pty opening error checking and handling Matthew Daley
2013-12-16 11:58         ` 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=529B2086.1080804@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=ian.campbell@citrix.com \
    --cc=ian.jackson@eu.citrix.com \
    --cc=mattd@bugfuzz.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).