diff mbox series

Handle Windows nul device in unlink-if-ordinary.c

Message ID 20230106083118.2141-1-anothername27-unity@yahoo.com
State Accepted
Headers
Series Handle Windows nul device in unlink-if-ordinary.c |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Li, Pan2 via Gcc-patches Jan. 6, 2023, 8:31 a.m. UTC 
  From: Himal <himalr@proton.me>

Hi,

This might be a better fix.

Regards.

PS. I had to use a different email.

---
 libiberty/unlink-if-ordinary.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Jeff Law March 11, 2023, 8:18 p.m. UTC | #1 
On 1/6/23 01:31, anothername27-unity--- via Gcc-patches wrote:
> From: Himal <himalr@proton.me>
> 
> Hi,
> 
> This might be a better fix.
> 
> Regards.
> 
> PS. I had to use a different email.
> 
> ---
>   libiberty/unlink-if-ordinary.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/libiberty/unlink-if-ordinary.c b/libiberty/unlink-if-ordinary.c
> index 84328b216..e765ac8b1 100644
> --- a/libiberty/unlink-if-ordinary.c
> +++ b/libiberty/unlink-if-ordinary.c
> @@ -62,6 +62,12 @@ was made to unlink the file because it is special.
>   int
>   unlink_if_ordinary (const char *name)
>   {
> +/* MS-Windows 'stat' function (and in turn, S_ISREG)
> +   reports the null device as a regular file.  */
> +#ifdef _WIN32
> +  if (stricmp (name, "nul") == 0)
> +    return 1;
> +#endif
Umm, wouldn't this return true for a real file called nul in the current 
directory?  ie, don't you need to distinguish between the nul device and 
a file named nul based on the full path?

And not being a windows person, I'd really like to see some 
documentation which indicates that stat on the null device will indicate 
its a regular file.  Alternately if one of the windows experts here can 
chime in, it'd be appreciated.
jeff
Himal March 13, 2023, 5:15 a.m. UTC | #2 
On 3/12/2023 1:48 AM, Jeff Law wrote:
>
>
> On 1/6/23 01:31, anothername27-unity--- via Gcc-patches wrote:
>> From: Himal <himalr@proton.me>
>>
>> Hi,
>>
>> This might be a better fix.
>>
>> Regards.
>>
>> PS. I had to use a different email.
>>
>> ---
>>   libiberty/unlink-if-ordinary.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/libiberty/unlink-if-ordinary.c 
>> b/libiberty/unlink-if-ordinary.c
>> index 84328b216..e765ac8b1 100644
>> --- a/libiberty/unlink-if-ordinary.c
>> +++ b/libiberty/unlink-if-ordinary.c
>> @@ -62,6 +62,12 @@ was made to unlink the file because it is special.
>>   int
>>   unlink_if_ordinary (const char *name)
>>   {
>> +/* MS-Windows 'stat' function (and in turn, S_ISREG)
>> +   reports the null device as a regular file.  */
>> +#ifdef _WIN32
>> +  if (stricmp (name, "nul") == 0)
>> +    return 1;
>> +#endif

Hi Jeff, Thanks for the response.

> Umm, wouldn't this return true for a real file called nul in the 
> current directory?  ie, don't you need to distinguish between the nul 
> device and a file named nul based on the full path?

I don't think that we can create a file called nul under Windows.

> And not being a windows person, I'd really like to see some 
> documentation which indicates that stat on the null device will 
> indicate its a regular file.  Alternately if one of the windows 
> experts here can chime in, it'd be appreciated.
> jeff

I found these patches that might indicate the same thing.

https://src.fedoraproject.org/rpms/binutils/blob/0b119dd9d51a3763db7d6fea1b51a03494cb96d8/f/binutils-CVE-2021-20197.patch#_121-135

https://github.com/msys2/MINGW-packages/pull/10541/files

I would like to see some input from a Windows developer as well.

BTW, This doesn't affecting anything. I stumbled upon this while 
debugging another 
[bug](https://sourceware.org/bugzilla/show_bug.cgi?id=29947). I noticed 
it's calling unlink function for the nul device as well, but it wasn't 
throwing any errors or anything like that.

Regards.
Jeff Law April 29, 2023, 6:10 p.m. UTC | #3 
On 3/12/23 23:15, Himal wrote:
> On 3/12/2023 1:48 AM, Jeff Law wrote:
>>
>>
>> On 1/6/23 01:31, anothername27-unity--- via Gcc-patches wrote:
>>> From: Himal <himalr@proton.me>
>>>
>>> Hi,
>>>
>>> This might be a better fix.
>>>
>>> Regards.
>>>
>>> PS. I had to use a different email.
>>>
>>> ---
>>>   libiberty/unlink-if-ordinary.c | 6 ++++++
>>>   1 file changed, 6 insertions(+)
>>>
>>> diff --git a/libiberty/unlink-if-ordinary.c 
>>> b/libiberty/unlink-if-ordinary.c
>>> index 84328b216..e765ac8b1 100644
>>> --- a/libiberty/unlink-if-ordinary.c
>>> +++ b/libiberty/unlink-if-ordinary.c
>>> @@ -62,6 +62,12 @@ was made to unlink the file because it is special.
>>>   int
>>>   unlink_if_ordinary (const char *name)
>>>   {
>>> +/* MS-Windows 'stat' function (and in turn, S_ISREG)
>>> +   reports the null device as a regular file.  */
>>> +#ifdef _WIN32
>>> +  if (stricmp (name, "nul") == 0)
>>> +    return 1;
>>> +#endif
> 
> Hi Jeff, Thanks for the response.
> 
>> Umm, wouldn't this return true for a real file called nul in the 
>> current directory?  ie, don't you need to distinguish between the nul 
>> device and a file named nul based on the full path?
> 
> I don't think that we can create a file called nul under Windows.
> 
>> And not being a windows person, I'd really like to see some 
>> documentation which indicates that stat on the null device will 
>> indicate its a regular file.  Alternately if one of the windows 
>> experts here can chime in, it'd be appreciated.
>> jeff
> 
> I found these patches that might indicate the same thing.
> 
> https://src.fedoraproject.org/rpms/binutils/blob/0b119dd9d51a3763db7d6fea1b51a03494cb96d8/f/binutils-CVE-2021-20197.patch#_121-135
> 
> https://github.com/msys2/MINGW-packages/pull/10541/files
> 
> I would like to see some input from a Windows developer as well.
> 
> BTW, This doesn't affecting anything. I stumbled upon this while 
> debugging another 
> [bug](https://sourceware.org/bugzilla/show_bug.cgi?id=29947). I noticed 
> it's calling unlink function for the nul device as well, but it wasn't 
> throwing any errors or anything like that.
I'm inclined to go ahead and commit this.  I think the only other 
question I have is the use of stricmp.  That's not strictly ISO, 
strcasecmp would be preferred.  But I don't know enough about the 
windows environment to know if they picked up strcasecmp over time.

jeff
diff mbox series

Patch

diff --git a/libiberty/unlink-if-ordinary.c b/libiberty/unlink-if-ordinary.c
index 84328b216..e765ac8b1 100644
--- a/libiberty/unlink-if-ordinary.c
+++ b/libiberty/unlink-if-ordinary.c
@@ -62,6 +62,12 @@  was made to unlink the file because it is special.
 int
 unlink_if_ordinary (const char *name)
 {
+/* MS-Windows 'stat' function (and in turn, S_ISREG)
+   reports the null device as a regular file.  */
+#ifdef _WIN32
+  if (stricmp (name, "nul") == 0)
+    return 1;
+#endif
   struct stat st;
 
   if (lstat (name, &st) == 0