public inbox for cygwin-developers@cygwin.com
 help / color / mirror / Atom feed
From: Corinna Vinschen <corinna-cygwin@cygwin.com>
To: cygwin-developers@cygwin.com
Subject: Re: cygwin_create_path causes null pointer crashes
Date: Tue, 13 Dec 2016 11:57:00 -0000	[thread overview]
Message-ID: <20161213115648.GA17377@calimero.vinschen.de> (raw)
In-Reply-To: <3a85ca92-2959-9963-1934-4e3f4e448cf1@towo.net>

[-- Attachment #1: Type: text/plain, Size: 3608 bytes --]

On Dec 12 20:28, Thomas Wolff wrote:
> Hi Corinna,
> 
> Am 11.12.2016 um 12:08 schrieb Corinna Vinschen:
> > Hi Thomas,
> > 
> > On Dec 10 03:03, Thomas Wolff wrote:
> > > cygwin_create_path is supposed to call malloc itself,
> > > however, it occasionally returns with null and errno == ENOSPC,
> > > if it's provided with a network path, e.g.
> > > cygwin_create_path(CCP_POSIX_TO_WIN_W, "/cygdrive/s/.config/mintty")
> > > returns null in ~ 3 of 1000 cases, where /cygdrive/s exists but
> > > /cygdrive/s/.config does not
> > Did you try to debug and fix the issue?  This is the developer's mailing
> > list after all...
> I can contribute some initial analysis:
> Cloning cygwin_create_path from cygwin/path.cc with these essentials:
>   ssize_t size = cygwin_conv_path (what, from, NULL, 0);
>   // checking size, calling malloc
>   if (cygwin_conv_path (what, from, to, size) == -1)

Cloning?  Why didn't you just debug with GDB and/or strace?

> there are actually two different errors happening, with example path
> "/cygdrive/s/.config/mintty":
> 1. the first call (for size determination) returns 44 but the second call
> returns the correct result "S:\.config\mintty".
> 2. the first call returns 36 but the second call (provided with a larger
> size value patched into the function) returns "S:\.config\mintty.lnk" (which
> needs 44 bytes); this is obviously the call that would return null if not
> patched with a faked buffer size

A simple strace shows that there's a really ugly problem with network
shares:

Consider that the Windows functions have two(*) error codes for
"file not found".

- STATUS_OBJECT_NAME_NOT_FOUND indicates that the path to the file is
  valid, but the file doesn't exist.

- STATUS_OBJECT_PATH_NOT_FOUND indicates that the path to the file
  doesn't exist.

I.e., if the .config dir exists, but mintty doesn't, NtOpenFile returns
STATUS_OBJECT_NAME_NOT_FOUND.  If the config doe doesn't exist,
NtOpenFile returns STATUS_OBJECT_PATH_NOT_FOUND.

The problem is that in case of network drives, the return code changes.

The first call to NtOpenFile ("S:\.config\mintty") returns
STATUS_OBJECT_PATH_NOT_FOUND.  The second and all subsequent calls
return STATUS_OBJECT_NAME_NOT_FOUND.

Apparently this is a caching issue.  If you wait a bit and repeat the
call, it returns STATUS_OBJECT_PATH_NOT_FOUND again.  This does not
occur with local paths.

This is *so* wrong.  And the downside is that Cygwin relies on the
correct return code in the path conversion code.  Oh well.

Anyway, we can't fix Windows, but we can workaround its problems, which
is what Cygwin does, so I applied a patch.

> Another observation, by the slowness of the calls, is that apparently the
> drive is actually accessed during the path conversion, which I wouldn't have
> expected from a plain path conversion function.

It has to since the existence check on files has to check for
.lnk and .exe suffixes and it has to return an error code, e.g.
ENOENT, ENOTDIR, EACCES, ...

> The underlying function cygwin_conv_path looks a bit complicated for a
> straight-forward analysis to me...

It only consists of a simple switch statement for the four conversions
POSIX <-> Windows (char/wchar_t).  All the dirty work is done by
path_conv::check.

I just generated a new developer snapshot and I'm going to create
a 2.6.1-0.2 test release.  Please test.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

  reply	other threads:[~2016-12-13 11:57 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-10  2:03 Thomas Wolff
2016-12-11 11:08 ` Corinna Vinschen
2016-12-12 19:28   ` Thomas Wolff
2016-12-13 11:57     ` Corinna Vinschen [this message]
2016-12-14 20:25       ` Thomas Wolff

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=20161213115648.GA17377@calimero.vinschen.de \
    --to=corinna-cygwin@cygwin.com \
    --cc=cygwin-developers@cygwin.com \
    /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 read-only IMAP folder(s) and NNTP newsgroup(s).