diff mbox

getpwnam(NULL) get "segmentation fault"

Message ID 20150530173322.GG2101@vapier
State Committed
Delegated to: Mike Frysinger
Headers show

Commit Message

Mike Frysinger May 30, 2015, 5:33 p.m. UTC
On 30 May 2015 16:00, Andrew Pinski wrote:
> On Sat, May 30, 2015 at 3:52 PM, Sheng Yong <shengyong1@huawei.com> wrote:
> > When using getpwnam(), we found that if the parameter is NULL, the program
> > would get a segmentation fault. I checked the glibc code, and found that
> > parameter passed to function is defined as a macro ADD_PARAMS and its value
> > is not checked before using.
> >
> > It seems serval functions under nss directory doing so. But segmentation
> > fault is not friendly to users. Maybe we could add some parameter-check
> > code before using theses parameters like the following. But I don't know
> > if it is reasonable to do that. Any hint is appreciated.
> 
> I don't see anywhere in POSIX standard where it says this is well
> defined.  We have many different places were we don't check for NULL
> pointers including and not limited to memcpy, strcmp, strcpy.  So in
> my mind, this should fall in the same idea.

agreed.  we should however make sure it has the right markings in the header.
i poked through the internals to make sure we deref these pointers w/out checks, 
but some of the funcs can be hard to trace (due to all the nss muck).
-mike

2015-05-31  Mike Frysinger  <vapier@gentoo.org>

	* pwd/pwd.h (fgetpwent): Add __nonnull markings.
	(putpwent): Likewise.
	(getpwnam): Likewise.
	(getpwent_r): Likewise.
	(getpwuid_r): Likewise.
	(getpwnam_r): Likewise.
	(fgetpwent_r): Likewise.

Comments

Roland McGrath June 5, 2015, 9 p.m. UTC | #1
> 2015-05-31  Mike Frysinger  <vapier@gentoo.org>
> 
> 	* pwd/pwd.h (fgetpwent): Add __nonnull markings.
> 	(putpwent): Likewise.
> 	(getpwnam): Likewise.
> 	(getpwent_r): Likewise.
> 	(getpwuid_r): Likewise.
> 	(getpwnam_r): Likewise.
> 	(fgetpwent_r): Likewise.

Looks good.
diff mbox

Patch

diff --git a/pwd/pwd.h b/pwd/pwd.h
index ff49564..fcfb2ab 100644
--- a/pwd/pwd.h
+++ b/pwd/pwd.h
@@ -91,7 +91,7 @@  extern struct passwd *getpwent (void);
    cancellation point.  But due to similarity with an POSIX interface
    or due to the implementation it is a cancellation point and
    therefore not marked with __THROW.  */
-extern struct passwd *fgetpwent (FILE *__stream);
+extern struct passwd *fgetpwent (FILE *__stream) __nonnull ((1));
 
 /* Write the given entry onto the given stream.
 
@@ -100,7 +100,7 @@  extern struct passwd *fgetpwent (FILE *__stream);
    or due to the implementation it is a cancellation point and
    therefore not marked with __THROW.  */
 extern int putpwent (const struct passwd *__restrict __p,
-		     FILE *__restrict __f);
+		     FILE *__restrict __f) __nonnull ((1, 2));
 #endif
 
 /* Search for an entry with a matching user ID.
@@ -113,7 +113,7 @@  extern struct passwd *getpwuid (__uid_t __uid);
 
    This function is a possible cancellation point and therefore not
    marked with __THROW.  */
-extern struct passwd *getpwnam (const char *__name);
+extern struct passwd *getpwnam (const char *__name) __nonnull ((1));
 
 #ifdef __USE_POSIX
 
@@ -138,18 +138,21 @@  extern struct passwd *getpwnam (const char *__name);
    therefore not marked with __THROW.  */
 extern int getpwent_r (struct passwd *__restrict __resultbuf,
 		       char *__restrict __buffer, size_t __buflen,
-		       struct passwd **__restrict __result);
+		       struct passwd **__restrict __result)
+		       __nonnull ((1, 2, 4));
 # endif
 
 extern int getpwuid_r (__uid_t __uid,
 		       struct passwd *__restrict __resultbuf,
 		       char *__restrict __buffer, size_t __buflen,
-		       struct passwd **__restrict __result);
+		       struct passwd **__restrict __result)
+		       __nonnull ((2, 3, 5));
 
 extern int getpwnam_r (const char *__restrict __name,
 		       struct passwd *__restrict __resultbuf,
 		       char *__restrict __buffer, size_t __buflen,
-		       struct passwd **__restrict __result);
+		       struct passwd **__restrict __result)
+		       __nonnull ((1, 2, 3, 5));
 
 
 # ifdef	__USE_MISC
@@ -163,7 +166,8 @@  extern int getpwnam_r (const char *__restrict __name,
 extern int fgetpwent_r (FILE *__restrict __stream,
 			struct passwd *__restrict __resultbuf,
 			char *__restrict __buffer, size_t __buflen,
-			struct passwd **__restrict __result);
+			struct passwd **__restrict __result)
+			__nonnull ((1, 2, 3, 5));
 # endif
 
 #endif	/* POSIX or reentrant */