On Thu, 1 May 2025, 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!
>
> > @@ -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...
I thought of that, but I wasn't sure if there was room to memmove the
contents over to prepend the ro_u_globalroot constant. I could check the
MaximumLength and if not reallocate Buffer but that would then require
knowledge of which allocation mechanism was used to ensure the matching
realloc or free is called. Either way, there's a lot more memory being
copied around.
> > +
> > +- 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" ?
Hmm, did you read the rust backtrace thread? The reviewer there was
concerned that the docs didn't specify that to could be NULL if size was
0, even though the example on that page does just that. It'd also be nice
to guarantee that to will always be NUL-terminated and never truncated.
I'd probably go with 'can be NULL', I don't want somebody to think that
it'd be a better idea to use (void *)8 :P
