[PATCH v4] Cygwin: respect PC_SYM_FOLLOW and PC_SYM_NOFOLLOW_REP with inner links
The new GetFinalPathNameW handling for native symlinks in inner path
components is disabled if caller doesn't want to follow symlinks, or
doesn't want to follow reparse points.
---
winsup/cygwin/path.cc | 88 ++-
1 file changed, 45 insertions(+), 43 deletions(-)
diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
index e62f8fe2b..1869fb8c8 100644
--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -722,9 +722,10 @@ path_conv::check (const char *src, unsigned opt,
int symlen = 0;
/* Make sure to check certain flags on last component only. */
- for (unsigned pc_flags = opt & (PC_NO_ACCESS_CHECK | PC_KEEP_HANDLE);
+ for (unsigned pc_flags = opt & (PC_NO_ACCESS_CHECK | PC_KEEP_HANDLE
+| PC_SYM_FOLLOW | PC_SYM_NOFOLLOW_REP);
;
- pc_flags = 0)
+ pc_flags = opt & (PC_SYM_FOLLOW | PC_SYM_NOFOLLOW_REP))
{
const suffix_info *suff;
char *full_path;
@@ -3480,48 +3481,49 @@ restart:
goto file_not_symlink;
}
#endif /* __i386__ */
- {
- PWCHAR fpbuf = tp.w_get ();
- DWORD ret;
-
- ret = GetFinalPathNameByHandleW (h, fpbuf, NT_MAX_PATH, 0);
- if (ret)
- {
- UNICODE_STRING fpath;
+ if ((pc_flags & (PC_SYM_FOLLOW | PC_SYM_NOFOLLOW_REP)) == PC_SYM_FOLLOW)
+ {
+ PWCHAR fpbuf = tp.w_get ();
+ DWORD ret;
- RtlInitCountedUnicodeString (, fpbuf, ret * sizeof (WCHAR));
- fpbuf[1] = L'?';/* \\?\ --> \??\ */
- if (!RtlEqualUnicodeString (, , !!ci_flag))
- {
- issymlink = true;
- /* upath.Buffer is big enough and unused from this point on.
- Reuse it here, avoiding yet another buffer allocation. */
- char *nfpath = (char *) upath.Buffer;
- sys_wcstombs (nfpath, NT_MAX_PATH, fpbuf);
- res = posixify (nfpath);
-
- /* If the incoming path consisted of a drive prefix only,
- we just handle a virtual drive, created with, e.g.
-
-subst X: C:\foo\bar
-
- Treat it like a symlink. This is required to tell an
- lstat caller that the "drive" is actually pointing
- somewhere else, thus, it's a symlink in POSIX speak. */
- if (upath.Length == 14) /* \??\X:\ */
- {
- fileattr &= ~FILE_ATTRIBUTE_DIRECTORY;
- path_flags |= PATH_SYMLINK;
- }
- /* For final paths differing in inner path components return
- length as negative value. This informs path_conv::check
- to skip realpath handling on the last path component. */
- else
- res = -res;
- break;
- }
- }
- }
+ ret = GetFinalPathNameByHandleW (h, fpbuf, NT_MAX_PATH, 0);
+ if (ret)
+ {
+ UNICODE_STRING fpath;
+
+ RtlInitCountedUnicodeString (, fpbuf, ret * sizeof (WCHAR));
+ fpbuf[1] = L'?'; /* \\?\ --> \??\ */
+ if (!RtlEqualUnicodeString (, , !!ci_flag))
+ {
+ issymlink = true;
+ /* upath.Buffer is big enough and unused from this point on.
+Reuse it here, avoiding yet another buffer allocation. */
+ char *nfpath = (char *) upath.Buffer;
+ sys_wcstombs (nfpath, NT_MAX_PATH, fpbuf);
+ res = posixify (nfpath);
+
+ /* If the incoming path consisted of a drive prefix only,
+we just handle a virtual drive, created with, e.g.
+
+ subst X: C:\foo\bar
+
+Treat it like a symlink. This is required to tell an
+lstat caller that the "drive" is actually pointing
+somewhere else, thus, it's a symlink in POSIX speak. */
+ if (upath.Length == 14) /* \??\X:\ */
+ {
+ fileattr &= ~FILE_ATTRIBUTE_DIRECTORY;
+ path_flags |= PATH_SYMLINK;
+ }
+ /* For final paths differing in inner path components return
+length as negative value. This informs path_conv::check
+to skip realpath handling on the last path component. */
+ else
+ res = -res;
+ break;
+ }
+ }
+ }
/* Normal file. */
file_not_symlink:
--
2.32.0.windows.1
From 67a276c35a3b48697c6b61caaf4ffea5a174c75b Mon Sep 17 00:00:00 2001
From: Jeremy Drake
Date: Sat, 29 May 2021 11:48:11 -0700
Subject: [PATCH] Cygwin: respect PC_SYM_FOLLOW and PC_SYM_NOFOLLOW_REP with
inner links.
The new
Re: [PATCH v3] Cygwin: respect PC_SYM_FOLLOW and PC_SYM_NOFOLLOW_REP with inner links
On Tue, 6 Jul 2021, Corinna Vinschen wrote:
> This formatting is just ugly. I suggest to move the PC_SYM_* test
> to the block after the 32 bit code and reuse the existing braces,
> just with adapted indentation, i. e.:
+1. I was trying to avoid reformatting otherwise unchanged lines to
reduce patch size.
> > @@ -3704,7 +3708,8 @@ chdir (const char *in_dir)
> >
> >/* Convert path. PC_NONULLEMPTY ensures that we don't check for
> > NULL/empty/invalid again. */
> > - path_conv path (in_dir, PC_SYM_FOLLOW | PC_POSIX | PC_NONULLEMPTY);
> > + path_conv path (in_dir, PC_SYM_FOLLOW | PC_POSIX | PC_NONULLEMPTY
> > + | PC_SYM_NOFOLLOW_REP);
> >if (path.error)
> > {
> > set_errno (path.error);
>
> I'm still not convinced that we should do this. I'm pretty certain this
> will result in problems in Cygwin processes when you least expect them.
>
> Consider that the output of getcwd and realpath/readlink on the same
> path may differ after this patch. Using PC_SYM_NOFOLLOW_REP like this
> also changes the normal sym follow handling for the last path component
> in path_copnv::check, potentially.
>
> This looks like here be dragons. A good solution would change the
> used native tools to allow paths > MAX_PATH finally, or to use other,
> equivalent tools already allowing that.
I am not convinced that this even completely solved the issues I was
seeing, or some of the reports of issues with unc paths suddenly showing
up instead of mapped drives in native tools that weren't expecting them.
But, I do think respecting the PC_SYM_NOFOLLOW_REP flag for inner links is
correct, and I am sending a new version.
Re: [PATCH v3] Cygwin: respect PC_SYM_FOLLOW and PC_SYM_NOFOLLOW_REP with inner links
On Jun 3 13:57, Jeremy Drake via Cygwin-patches wrote:
> [...]
> The new GetFinalPathNameW handling for native symlinks in inner path
> components is disabled if caller doesn't want to follow symlinks, or
> doesn't want to follow reparse points. Set flag to not follow reparse
> points in chdir, allowing native processes to see their cwd potentially
> including native symlinks, rather than dereferencing them.
> ---
> winsup/cygwin/path.cc | 11 ---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
> index e62f8fe2b..a6bb3aeff 100644
> --- a/winsup/cygwin/path.cc
> +++ b/winsup/cygwin/path.cc
> @@ -722,9 +722,10 @@ path_conv::check (const char *src, unsigned opt,
> int symlen = 0;
>
> /* Make sure to check certain flags on last component only. */
> - for (unsigned pc_flags = opt & (PC_NO_ACCESS_CHECK | PC_KEEP_HANDLE);
> + for (unsigned pc_flags = opt & (PC_NO_ACCESS_CHECK | PC_KEEP_HANDLE
> + | PC_SYM_FOLLOW | PC_SYM_NOFOLLOW_REP);
> ;
> -pc_flags = 0)
> +pc_flags = opt & (PC_SYM_FOLLOW | PC_SYM_NOFOLLOW_REP))
> {
> const suffix_info *suff;
> char *full_path;
> @@ -3452,6 +3453,8 @@ restart:
> break;
> }
>
> + if ((pc_flags & (PC_SYM_FOLLOW | PC_SYM_NOFOLLOW_REP)) ==
> PC_SYM_FOLLOW)
> + {
>/* Check if the inner path components contain native symlinks or
>junctions, or if the drive is a virtual drive. Compare incoming
>path with path returned by GetFinalPathNameByHandleA. If they
> @@ -3522,6 +3525,7 @@ restart:
> }
> }
>}
> + }
This formatting is just ugly. I suggest to move the PC_SYM_* test
to the block after the 32 bit code and reuse the existing braces,
just with adapted indentation, i. e.:
diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
index 6a07f0d06850..cb0386e24005 100644
--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -3480,43 +3480,44 @@ restart:
goto file_not_symlink;
}
#endif /* __i386__ */
- {
- PWCHAR fpbuf = tp.w_get ();
- DWORD ret;
+ if ((pc_flags & (PC_SYM_FOLLOW | PC_SYM_NOFOLLOW_REP)) == PC_SYM_FOLLOW)
+ {
+ PWCHAR fpbuf = tp.w_get ();
+ DWORD ret;
[...indent all lines inside the block accordingly...]
- }
+ }
/* Normal file. */
file_not_symlink:
> @@ -3704,7 +3708,8 @@ chdir (const char *in_dir)
>
>/* Convert path. PC_NONULLEMPTY ensures that we don't check for
> NULL/empty/invalid again. */
> - path_conv path (in_dir, PC_SYM_FOLLOW | PC_POSIX | PC_NONULLEMPTY);
> + path_conv path (in_dir, PC_SYM_FOLLOW | PC_POSIX | PC_NONULLEMPTY
> + | PC_SYM_NOFOLLOW_REP);
>if (path.error)
> {
> set_errno (path.error);
I'm still not convinced that we should do this. I'm pretty certain this
will result in problems in Cygwin processes when you least expect them.
Consider that the output of getcwd and realpath/readlink on the same
path may differ after this patch. Using PC_SYM_NOFOLLOW_REP like this
also changes the normal sym follow handling for the last path component
in path_copnv::check, potentially.
This looks like here be dragons. A good solution would change the
used native tools to allow paths > MAX_PATH finally, or to use other,
equivalent tools already allowing that.
Patch 1 is ok, of course. I pushed it.
Thanks,
Corinna
Re: propagate font zoom via SIGWINCH
Hi Thomas, On Jul 3 18:19, Thomas Wolff wrote: > xterm 368 and mintty 3.5.1 implement a new feature to support notification > of terminal scaling via font zooming also if the terminal text dimensions > (rows/columns) stay unchanged, using ioctl(TIOCSWINSZ), raising SIGWINCH. > This does not work in cygwin currently. The attached patch fixes that. > Thomas Can you please put the describing text into the commit message? Thanks, Corinna
Re: [PATCH] format_proc_cpuinfo: add Linux 5.13 AMD/Hygon rapl
On Jun 29 11:09, Brian Inglis wrote: > > Linux 5.13 Opossums on Parade added features and changes: > add AMD 0x8007 EDX:14 rapl runtime average power limit > --- > winsup/cygwin/fhandler_proc.cc | 8 > 1 file changed, 8 insertions(+) Pushed. Thanks, Corinna
Re: [PATCH v2] Cygwin: console: Fix garbled input for non-ASCII chars.
On Jun 24 12:40, Takashi Yano wrote: > - After the commit ff4440fc, non-ASCII input may sometimes be garbled. > This patch fixes the issue. > > Addresses: https://cygwin.com/pipermail/cygwin/2021-June/248775.html > --- > winsup/cygwin/fhandler_console.cc | 15 +-- > 1 file changed, 9 insertions(+), 6 deletions(-) Pushed. Apologies for applying the v1 first, accidentally! Thanks, Corinna
Re: [PATCH] Cygwin: console: Fix garbled input for non-ASCII chars.
On Jun 23 17:42, Takashi Yano wrote: > - After the commit ff4440fc, non-ASCII input may sometimes be garbled. > This patch fixes the issue. > > Addresses: https://cygwin.com/pipermail/cygwin/2021-June/248775.html > --- > winsup/cygwin/fhandler_console.cc | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Pushed. Thanks, Corinna
Re: [PATCH 0/2] Improve utils manpages
On Jun 16 17:19, Jon Turney wrote: > Jon Turney (2): > Cygwin: Various minor fixes to utils documentation > Cygwin: Use cmdsynopsis element in utils documentation > > winsup/doc/utils.xml | 709 --- > 1 file changed, 594 insertions(+), 115 deletions(-) > > -- > 2.31.1 Great stuff, please push. Thanks, Corinna
Re: [PATCH] Define PSAPI_VERSION as 1 before including psapi.h
Hi Jon, On Jun 20 14:37, Jon Turney wrote: > The default PSAPI_VERSION is controlled by WIN32_WINNT, which we set to > 0x0a00 when building utils since 48a76190 (and is the default in w32api > >= 9.0.0) > > In order for the built executables to run on Windows Vista, we must also > define PSAPI_VERSION as 1 (otherwise '#define GetModuleFileNameExA > K32GetModuleFileNameExA' causes a 'The procedure entry point > K32GetModuleFilenameExA could not be located in the dynamic link library > kernel32.dll' error at run time). > > Also drop uneeded psapi.h from dlfcn.cc (31ddf45d), resource.cc > (34a6eeab) and ps.cc (1def2148). > --- > winsup/cygwin/dlfcn.cc | 1 - > winsup/cygwin/resource.cc | 1 - > winsup/utils/dumper.cc | 1 + > winsup/utils/ldd.cc | 2 +- > winsup/utils/mingw/bloda.cc | 1 + > winsup/utils/module_info.cc | 1 + > winsup/utils/pldd.c | 1 + > winsup/utils/ps.cc | 1 - > 8 files changed, 5 insertions(+), 4 deletions(-) +1. You really don't have to wait for me to push stuff like that. Thanks, Corinna
