All of lore.kernel.org
 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-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
2013-12-02 11:51       ` [PATCH] xenconsole: merge pty access check into when it is opened Ian Jackson
2013-12-02 11:53         ` Andrew Cooper

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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.