[PATCH] Cygwin: path_conv::eq_worker: add NULL pointer checks

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Cygwin: path_conv::eq_worker: add NULL pointer checks

cygwin-patches mailing list
Don't call cstrdup on NULL pointers.
---
 winsup/cygwin/path.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/winsup/cygwin/path.h b/winsup/cygwin/path.h
index b94f13df8..0b3e72fc1 100644
--- a/winsup/cygwin/path.h
+++ b/winsup/cygwin/path.h
@@ -320,9 +320,11 @@ class path_conv
        contrast to statically allocated strings.  Calling device::dup()
        will duplicate the string if the source was allocated. */
     dev.dup ();
-    path = cstrdup (in_path);
+    if (in_path)
+      path = cstrdup (in_path);
     conv_handle.dup (pc.conv_handle);
-    posix_path = cstrdup(pc.posix_path);
+    if (pc.posix_path)
+      posix_path = cstrdup(pc.posix_path);
     if (pc.wide_path)
       {
  wide_path = cwcsdup (uni_path.Buffer);
--
2.29.2

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: path_conv::eq_worker: add NULL pointer checks

Corinna Vinschen-2
On Nov 14 09:16, Ken Brown via Cygwin-patches wrote:

> Don't call cstrdup on NULL pointers.
> ---
>  winsup/cygwin/path.h | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/winsup/cygwin/path.h b/winsup/cygwin/path.h
> index b94f13df8..0b3e72fc1 100644
> --- a/winsup/cygwin/path.h
> +++ b/winsup/cygwin/path.h
> @@ -320,9 +320,11 @@ class path_conv
>         contrast to statically allocated strings.  Calling device::dup()
>         will duplicate the string if the source was allocated. */
>      dev.dup ();
> -    path = cstrdup (in_path);
> +    if (in_path)
> +      path = cstrdup (in_path);
>      conv_handle.dup (pc.conv_handle);
> -    posix_path = cstrdup(pc.posix_path);
> +    if (pc.posix_path)
> +      posix_path = cstrdup(pc.posix_path);
>      if (pc.wide_path)
>        {
>   wide_path = cwcsdup (uni_path.Buffer);
> --
> 2.29.2

LGTM.  How did you notice this?  Maybe a pointer to the problem
in the log message may be helpful in future.


Thanks,
Corinna
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: path_conv::eq_worker: add NULL pointer checks

cygwin-patches mailing list
On 11/16/2020 7:08 AM, Corinna Vinschen wrote:

> On Nov 14 09:16, Ken Brown via Cygwin-patches wrote:
>> Don't call cstrdup on NULL pointers.
>> ---
>>   winsup/cygwin/path.h | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/winsup/cygwin/path.h b/winsup/cygwin/path.h
>> index b94f13df8..0b3e72fc1 100644
>> --- a/winsup/cygwin/path.h
>> +++ b/winsup/cygwin/path.h
>> @@ -320,9 +320,11 @@ class path_conv
>>          contrast to statically allocated strings.  Calling device::dup()
>>          will duplicate the string if the source was allocated. */
>>       dev.dup ();
>> -    path = cstrdup (in_path);
>> +    if (in_path)
>> +      path = cstrdup (in_path);
>>       conv_handle.dup (pc.conv_handle);
>> -    posix_path = cstrdup(pc.posix_path);
>> +    if (pc.posix_path)
>> +      posix_path = cstrdup(pc.posix_path);
>>       if (pc.wide_path)
>>         {
>>   wide_path = cwcsdup (uni_path.Buffer);
>> --
>> 2.29.2
>
> LGTM.  How did you notice this?  Maybe a pointer to the problem
> in the log message may be helpful in future.

I noticed it in my attempts to do fhandler serialization/deserialization as
we've discussed on cygwin-developers.  I was getting a crash when cloning an
fhandler whose pc member had freed its strings.  I'll add something to the log
message.

Ken