From patchwork Sat May 30 17:33:22 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mike Frysinger X-Patchwork-Id: 6987 X-Patchwork-Delegate: vapier@gentoo.org Received: (qmail 47606 invoked by alias); 30 May 2015 17:33:29 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 47596 invoked by uid 89); 30 May 2015 17:33:28 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL, BAYES_00, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: smtp.gentoo.org Date: Sat, 30 May 2015 13:33:22 -0400 From: Mike Frysinger To: Andrew Pinski Cc: Sheng Yong , Roland McGrath , GNU C Library Subject: Re: getpwnam(NULL) get "segmentation fault" Message-ID: <20150530173322.GG2101@vapier> Mail-Followup-To: Andrew Pinski , Sheng Yong , Roland McGrath , GNU C Library References: <55696C2D.5040200@huawei.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: On 30 May 2015 16:00, Andrew Pinski wrote: > On Sat, May 30, 2015 at 3:52 PM, Sheng Yong 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 * pwd/pwd.h (fgetpwent): Add __nonnull markings. (putpwent): Likewise. (getpwnam): Likewise. (getpwent_r): Likewise. (getpwuid_r): Likewise. (getpwnam_r): Likewise. (fgetpwent_r): Likewise. 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 */