Bug 1018331

Summary: strdup(3) segfaults when passing NULL to it
Product: Red Hat Enterprise Linux 7 Reporter: Eryu Guan <eguan>
Component: glibcAssignee: Carlos O'Donell <codonell>
Status: CLOSED NOTABUG QA Contact: qe-baseos-tools-bugs
Severity: high Docs Contact:
Priority: high    
Version: 7.0CC: ashankar, fweimer, mfranc, mpolacek, pfrankli, spoyarek
Target Milestone: rc   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: 1018323 Environment:
Last Closed: 2013-10-12 11:21:27 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 1018323    
Bug Blocks: 1018326    

Description Eryu Guan 2013-10-11 17:31:03 UTC
glibc-2.17-33.el7 has this issue too

+++ This bug was initially created as a clone of Bug #1018323 +++

Description of problem:
$ cat strduptest.c 
#include <string.h>

int main(void)
{
        strdup(NULL);
}

$ gcc -o strduptest strduptest.c
$ ./strduptest
Segmentation fault (core dumped)

And dmesg shows:
strduptest[2374]: segfault at 0 ip 0000003b97086711 sp 00007fff67a8cd98 error 4 in libc-2.17.so[3b97000000+1b6000]

Version-Release number of selected component (if applicable):
glibc-2.17-18.fc19

How reproducible:
always

Steps to Reproduce:
1. see description
2.
3.

Actual results:
segfault

Expected results:
strdup(3) should deal with NULL correctly

Additional info:

Comment 1 Siddhesh Poyarekar 2013-10-12 01:15:11 UTC
Don't pass NULL to strdup or any string functions since the spec does not allow them:

http://pubs.opengroup.org/onlinepubs/9699919799/functions/strdup.html

Comment 2 Eryu Guan 2013-10-12 04:05:16 UTC
It's surely an application bug to pass NULL to strdup, but shouldn't the str* functions deal with invalid arguments more gracefully? Rather than simply giving a segfault in libc to user. I'm not sure...

Comment 3 Marek Polacek 2013-10-12 11:21:27 UTC
I think strdup just in turn calls memcpy and if the source pointer is not valid, the behavior is undefined.  Closing.

Comment 4 Carlos O'Donell 2013-10-15 18:41:45 UTC
(In reply to Eryu Guan from comment #2)
> It's surely an application bug to pass NULL to strdup, but shouldn't the
> str* functions deal with invalid arguments more gracefully? Rather than
> simply giving a segfault in libc to user. I'm not sure...

No.

The GNU C Library specifically does not check for NULL pointers and we document our policy in "Style and Conventions" -> "Invalid Pointers" on our project wiki:

https://sourceware.org/glibc/wiki/Style_and_Conventions#Invalid_pointers

~~~
9. Invalid pointers

The GNU C library considers it a QoI feature not to mask user bugs by detecting invalid pointers and returning EINVAL (unless the API is standardized and says it does that). If passing a bad pointer has undefined behavior, it is far more useful in the long run if it crashes quickly rather than diagnosing an error that is probably ignored by the flaky caller.

9.1. NULL pointers

If you're going to check for NULL pointer arguments where you have not entered into a contract to accept and interpret them, do so with an assert, not a conditional error return. This way the bugs in the caller will be immediately detected and can be fixed, and it makes it easy to disable the overhead in production builds. The assert can be valuable as code documentation. However, a segfault from dereferencing the NULL pointer is just as effective for debugging. If you return an error code to a caller which has already proven itself buggy, the most likely result is that the caller will ignore the error, and bad things will happen much later down the line when the original cause of the error has become difficult or impossible to track down. Why is it reasonable to assume the caller will ignore the error you return? Because the caller already ignored the error return of malloc or fopen or some other library-specific allocation function which returned NULL to indicate an error.

In summary:

    * If you have no contract to accept NULL and you don't immediately dereference the pointer then use an assert to raise an error when NULL is passed as an invalid argument.

    * If you have no contract to accept NULL and immediately dereference the pointer then the segfault is sufficient to indicate the error.

    * If you have a contract to accept NULL then do so. 
~~~