On Thu, 1 May 2025 12:55:40 +0100
Jon Turney <wrote:
> On 29/04/2025 18:42, Jeremy Drake via Cygwin-patches wrote:
> > In the CCP_POSIX_TO_WIN_W path, when `from` is a device,
> > cygwin_conv_path would attempt to write to the `to` buffer before the
> > validation of the `size`. This resulted in an EFAULT error in the
> > common use-case of passing `to` as NULL and `size` as 0 to get the
> > required size of `to` for the conversion (as used in
>
> This is clearly not what's wanted! Thanks for fixing this!
Pushed.
> > cygwin_create_path). Instead, set a boolean and write to `to`
> > after validation.
> >
> > Fixes: 43f65cdd7dae ("* Makefile.in (DLL_OFILES): Add fhandler_procsys.o.")
> > Addresses: https://cygwin.com/pipermail/cygwin/2025-April/258068.html
> > Signed-off-by: Jeremy Drake <[email protected]>
> > ---
> > winsup/cygwin/path.cc | 5 ++++-
> > winsup/cygwin/release/3.6.2 | 3 +++
> > 2 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
> > index 7a08e978ad..d26f99ee7f 100644
> > --- a/winsup/cygwin/path.cc
> > +++ b/winsup/cygwin/path.cc
> > @@ -3911,6 +3911,7 @@ cygwin_conv_path (cygwin_conv_path_t what, const void
> > *from, void *to,
> > int how = what & CCP_CONVFLAGS_MASK;
> > what &= CCP_CONVTYPE_MASK;
> > int ret = -1;
> > + bool prependglobalroot = false;
> >
> > __try
> > {
> > @@ -4019,7 +4020,7 @@ cygwin_conv_path (cygwin_conv_path_t what, const void
> > *from, void *to,
> > {
> > /* Device name points to somewhere else in the NT namespace.
> > Use GLOBALROOT prefix to convert to Win32 path. */
> > - to = (void *) wcpcpy ((wchar_t *) to, ro_u_globalroot.Buffer);
> > + prependglobalroot = true;
>
> It seems like this could all be done in-place in .Buffer here, but I'm
> going to defer to Corinna on if that's at all clearer...
>
> > lsiz += ro_u_globalroot.Length / sizeof (WCHAR);
> > }
> > /* TODO: Same ".\\" band-aid as in CCP_POSIX_TO_WIN_A case. */
> > @@ -4075,6 +4076,8 @@ cygwin_conv_path (cygwin_conv_path_t what, const void
> > *from, void *to,
> > stpcpy ((char *) to, buf);
> > break;
> > case CCP_POSIX_TO_WIN_W:
> > + if (prependglobalroot)
> > + to = (void *) wcpcpy ((PWCHAR) to, ro_u_globalroot.Buffer);
> > wcpcpy ((PWCHAR) to, path);
> > break;
> > }
> > diff --git a/winsup/cygwin/release/3.6.2 b/winsup/cygwin/release/3.6.2
> > index bceabcab34..de6eae13fc 100644
> > --- a/winsup/cygwin/release/3.6.2
> > +++ b/winsup/cygwin/release/3.6.2
> > @@ -13,3 +13,6 @@ Fixes:
> >
> > - Fix setting DOS attributes on devices.
> > Addresse: https://cygwin.com/pipermail/cygwin/2025-April/257940.html
> > +
> > +- Fix cygwin_conv_path writing to 'to' pointer before size is checked.
> > + Addresses: https://cygwin.com/pipermail/cygwin/2025-April/258068.html
>
>
> Seems like this should also touch:
>
> https://cygwin.com/cygwin-api/func-cygwin-conv-path.html
>
> (source in winsup/doc/path.xml)
>
>
> I'm not sure what the conventional language to use for this common
> behaviour:
>
> "If size is 0, (to is ignored|to can be NULL) and cygwin_conv_path just
> returns the required buffer size in bytes" ?
Jon,
Could you please check if the patch
https://cygwin.com/pipermail/cygwin-patches/2025q2/013694.html
from Jeremy is as you intended?
--
Takashi Yano <[email protected]>
