GNU bug report logs - #79336
[PATCH] df: fix potential null pointer dereference

Previous Next

To reply to this bug, email your comments to 79336 AT debbugs.gnu.org.
There is no need to reopen the bug first.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: yubiao hu <huyubiaox <at> gmail.com>
To: bug-coreutils <at> gnu.org
Subject: [PATCH] df: fix potential null pointer dereference
Date: Fri, 29 Aug 2025 09:45:46 +0800

* src/df.c (get_dev): Fix potential null pointer dereference
- Avoid dereferencing stat_file when both device and
 mount_point are NULL
- Handle allocation failure for cell when mount_point
 is NULL

---
 src/df.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/src/df.c b/src/df.c
index fe2e0e3..f60319c 100644
--- a/src/df.c
+++ b/src/df.c
@@ -1050,7 +1050,7 @@ get_dev (char const *device, char const
*mount_point, char const *file,
         }
       else
         {
-          error (0, errno, "%s", quotef (stat_file));
+          error (0, errno, "%s", quotef (stat_file ? stat_file : "-"));
           exit_status = EXIT_FAILURE;
           return;
         }
@@ -1215,17 +1215,23 @@ get_dev (char const *device, char const
*mount_point, char const *file,
           break;

         case TARGET_FIELD:
+          {
+            if (! mount_point) {
+              cell = xstrdup ("-");
+              break;
+            }
 #ifdef HIDE_AUTOMOUNT_PREFIX
-          /* Don't print the first directory name in MOUNT_POINT if it's an
-             artifact of an automounter.  This is a bit too aggressive to be
-             the default.  */
-          if (STRNCMP_LIT (mount_point, "/auto/") == 0)
-            mount_point += 5;
-          else if (STRNCMP_LIT (mount_point, "/tmp_mnt/") == 0)
-            mount_point += 8;
+            /* Don't print the first directory name in MOUNT_POINT if it's an
+              artifact of an automounter.  This is a bit too aggressive to be
+              the default.  */
+            if (STRNCMP_LIT (mount_point, "/auto/") == 0)
+              mount_point += 5;
+            else if (STRNCMP_LIT (mount_point, "/tmp_mnt/") == 0)
+              mount_point += 8;
 #endif
-          cell = xstrdup (mount_point);
-          break;
+            cell = xstrdup (mount_point);
+            break;
+          }

         case INVALID_FIELD:
         default:
-- 
2.33.0

Message #8 received at 79336 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: yubiao hu <huyubiaox <at> gmail.com>
Cc: 79336 <at> debbugs.gnu.org
Subject: Re: bug#79336: [PATCH] df: fix potential null pointer dereference
Date: Sat, 30 Aug 2025 10:41:24 -0700

[Message part 1 (text/plain, inline)]

On 2025-08-28 18:45, yubiao hu wrote:
> * src/df.c (get_dev): Fix potential null pointer dereference
> - Avoid dereferencing stat_file when both device and
>   mount_point are NULL
> - Handle allocation failure for cell when mount_point
>   is NULL

Why is this patch needed? Can you give an example df invocation in which 
mount_point is null there? As far as I can see, that cannot happen.

Did your bug report come from static analysis? If so, which static 
analyzer did you use and how did you use it? Does the attached patch 
pacify your static analyzer?

[0001-df-pacify-static-analysis.patch (text/x-patch, attachment)]

Message #13 received at 79336-done <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: yubiao hu <huyubiaox <at> gmail.com>, 79336-done <at> debbugs.gnu.org
Subject: Re: bug#79336: [PATCH] df: fix potential null pointer dereference
Date: Sat, 30 Aug 2025 18:52:40 +0100

[Message part 1 (text/plain, inline)]

On 29/08/2025 02:45, yubiao hu wrote:
> * src/df.c (get_dev): Fix potential null pointer dereference
> - Avoid dereferencing stat_file when both device and
>   mount_point are NULL
> - Handle allocation failure for cell when mount_point
>   is NULL

These are valid concerns.
I also see potential null dereferences of device.
The attached patch takes a different approach
by ensuring arguments are initialized earlier.

Marking this as done.

Will push the attached later.

cheers,
Padraig

[df-null-pointers.patch (text/x-patch, attachment)]

Message #16 received at 79336 <at> debbugs.gnu.org (full text, mbox):

From: Collin Funk <collin.funk1 <at> gmail.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 79336 <at> debbugs.gnu.org, yubiao hu <huyubiaox <at> gmail.com>
Subject: Re: bug#79336: [PATCH] df: fix potential null pointer dereference
Date: Sat, 30 Aug 2025 11:07:51 -0700

Paul Eggert <eggert <at> cs.ucla.edu> writes:

> On 2025-08-28 18:45, yubiao hu wrote:
>> * src/df.c (get_dev): Fix potential null pointer dereference
>> - Avoid dereferencing stat_file when both device and
>>   mount_point are NULL
>> - Handle allocation failure for cell when mount_point
>>   is NULL
>
> Why is this patch needed? Can you give an example df invocation in
> which mount_point is null there? As far as I can see, that cannot
> happen.

That was my understanding as well. Since preceding that section there
is:

    /* If MOUNT_POINT is null, then the file system is not mounted, and this
       program reports on the file system that the special file is on.
       It would be better to report on the unmounted file system,
       but statfs doesn't do that on most systems.  */
    if (!stat_file)
      stat_file = mount_point ? mount_point : device;

I don't see why we would have a mount entry if both MOUNT_POINT and
DEVICE are NULL.

Collin

Message #19 received at 79336 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: yubiao hu <huyubiaox <at> gmail.com>, 79336 <at> debbugs.gnu.org
Subject: Re: bug#79336: [PATCH] df: fix potential null pointer dereference
Date: Sat, 30 Aug 2025 19:32:10 +0100

On 30/08/2025 18:52, Pádraig Brady wrote:
> On 29/08/2025 02:45, yubiao hu wrote:
>> * src/df.c (get_dev): Fix potential null pointer dereference
>> - Avoid dereferencing stat_file when both device and
>>    mount_point are NULL
>> - Handle allocation failure for cell when mount_point
>>    is NULL
> 
> These are valid concerns.
> I also see potential null dereferences of device.
> The attached patch takes a different approach
> by ensuring arguments are initialized earlier.

I'll hold off on pushing this since until Paul's questions are answered.
I did also think the issue was theoretical,
so my changes should also be seen as just pacifying static analysis,
and making the code a bit more defensive.

cheers,
Padraig

Message #22 received at 79336 <at> debbugs.gnu.org (full text, mbox):

From: yubiao hu <huyubiaox <at> gmail.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 79336 <at> debbugs.gnu.org
Subject: Re: bug#79336: [PATCH] df: fix potential null pointer dereference
Date: Mon, 1 Sep 2025 09:58:52 +0800

> On 2025-08-28 18:45, yubiao hu wrote:
>> * src/df.c (get_dev): Fix potential null pointer dereference
>> - Avoid dereferencing stat_file when both device and
>>   mount_point are NULL
>> - Handle allocation failure for cell when mount_point
>>   is NULL
>
> Why is this patch needed? Can you give an example df invocation in which
> mount_point is null there? As far as I can see, that cannot happen.
>
> Did your bug report come from static analysis? If so, which static
> analyzer did you use and how did you use it? Does the attached patch
> pacify your static analyzer?

Yes, this bug was identified via static code analysis. The initial
finding that core dump would occur in `cell = xstrdup (mount_point);`
when mount_point is NULL.

I attempted to inject code to set the mount_point of get_dev() to
NULL, which still results in a core within IS_ABSOLUTE_FILE_NAME.

Message #25 received at 79336 <at> debbugs.gnu.org (full text, mbox):

From: yubiao hu <huyubiaox <at> gmail.com>
To: "P <at> draigBrady.com" <P <at> draigbrady.com>
Subject: Re: bug#79336: [PATCH] df: fix potential null pointer dereference
Date: Mon, 1 Sep 2025 10:12:23 +0800

> On 29/08/2025 02:45, yubiao hu wrote:
>> * src/df.c (get_dev): Fix potential null pointer dereference
>> - Avoid dereferencing stat_file when both device and
>>   mount_point are NULL
>> - Handle allocation failure for cell when mount_point
>>   is NULL
>
> These are valid concerns.
> I also see potential null dereferences of device.
> The attached patch takes a different approach
> by ensuring arguments are initialized earlier.
>
> Marking this as done.
>
> Will push the attached later.

Thank you for the code optimization. I would like to highlight a
potential issue: if both mount_point and device are NULL, stat_file
still end up being NULL. This could lead to a core dump at the
`error(0, errno, "%s", quotef(stat_file));`

However, there may never be a situation where both mount_point and
device are null at the same time.

Message #28 received at 79336 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: yubiao hu <huyubiaox <at> gmail.com>
Cc: 79336 <at> debbugs.gnu.org
Subject: Re: bug#79336: [PATCH] df: fix potential null pointer dereference
Date: Sun, 31 Aug 2025 19:28:23 -0700

On 8/31/25 18:58, yubiao hu wrote:
> Yes, this bug was identified via static code analysis. The initial
> finding that core dump would occur in `cell = xstrdup (mount_point);`
> when mount_point is NULL.

It appears that the static analysis is reporting a false positive. Which 
static analyzer are you using? Or are you doing this by hand?

Do you get a false positive if you install the patch I sent you earlier? 
You can find that patch again here:

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=79336#8

Message #31 received at 79336 <at> debbugs.gnu.org (full text, mbox):

From: yubiao hu <huyubiaox <at> gmail.com>
To: "eggert <at> cs.ucla.edu" <eggert <at> cs.ucla.edu>
Cc: 79336 <at> debbugs.gnu.org
Subject: Re: bug#79336: [PATCH] df: fix potential null pointer dereference
Date: Mon, 1 Sep 2025 20:50:25 +0800

> On 8/31/25 18:58, yubiao hu wrote:
>> Yes, this bug was identified via static code analysis. The initial
>> finding that core dump would occur in `cell = xstrdup (mount_point);`
>> when mount_point is NULL.
>
> It appears that the static analysis is reporting a false positive. Which
> static analyzer are you using? Or are you doing this by hand?
>
> Do you get a false positive if you install the patch I sent you earlier?
> You can find that patch again here:
>
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=79336#8

The analysis was conducted using our internally-developed static
analysis tool. My apologies, but I am unable to share the tool itself
or its usage methodology.

I have tested the patch, and it can pacify my static analyzer.

Message #34 received at 79336-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: yubiao hu <huyubiaox <at> gmail.com>
Cc: 79336-done <at> debbugs.gnu.org
Subject: Re: bug#79336: [PATCH] df: fix potential null pointer dereference
Date: Mon, 1 Sep 2025 10:07:21 -0700

[Message part 1 (text/plain, inline)]

On 2025-09-01 05:50, yubiao hu wrote:
> I have tested the patch, and it can pacify my static analyzer.

Thanks for checking. I installed the attached patch, which is the same 
except with a comment fixed.

It's typically better not to patch merely to pacify a false positive 
from static analysis, if the patch introduces makework code that 
complicates the work of humans (and of other static analyzers) and that 
slows down execution. But here the patch streamlines the code (and 
caused me to review and fix commentary errors) so it's a clear win. 
Thanks for reporting the issue.

Closing the bug report.

[0001-df-pacify-static-analysis.patch (text/x-patch, attachment)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.