NetBSD Problem Report #51039

From www@NetBSD.org  Sun Apr  3 00:05:10 2016
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id C48617A490
	for <gnats-bugs@gnats.NetBSD.org>; Sun,  3 Apr 2016 00:05:09 +0000 (UTC)
Message-Id: <20160403000508.CDAA47AA98@mollari.NetBSD.org>
Date: Sun,  3 Apr 2016 00:05:08 +0000 (UTC)
From: er.abhinav.upadhyay@gmail.com
Reply-To: er.abhinav.upadhyay@gmail.com
To: gnats-bugs@NetBSD.org
Subject: makemandb(8): Check for return value of chdir(2)
X-Send-Pr-Version: www-1.0

>Number:         51039
>Category:       bin
>Synopsis:       makemandb(8): Check for return value of chdir(2)
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    bin-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Apr 03 00:10:00 +0000 2016
>Closed-Date:    Wed Apr 13 07:53:17 +0000 2016
>Last-Modified:  Wed Apr 13 07:53:17 +0000 2016
>Originator:     Abhinav Upadhyay
>Release:        CURRENT
>Organization:
>Environment:
>Description:
makemandb makes a call to chdir(2) before parsing each page but the return value of chdir is not checked. We should check the return status and handle the error.


>How-To-Repeat:

>Fix:
Index: makemandb.c
===================================================================
RCS file: /cvsroot/src/usr.sbin/makemandb/makemandb.c,v
retrieving revision 1.33
diff -u -r1.33 makemandb.c
--- makemandb.c	31 Mar 2016 20:17:58 -0000	1.33
+++ makemandb.c	3 Apr 2016 00:02:20 -0000
@@ -842,12 +842,19 @@
 			 * This means is either a new file or an updated file.
 			 * We should go ahead with parsing.
 			 */
+			if (chdir(parent) != 0) {
+				if (mflags.verbosity)
+					warn("chdir failed for %s, could not index %s", parent, file);
+				err_count++;
+				free(md5sum);
+				continue;
+			}
+
 			if (mflags.verbosity == 2)
 				printf("Parsing: %s\n", file);
 			rec->md5_hash = md5sum;
 			rec->file_path = estrdup(file);
 			// file_path is freed by insert_into_db itself.
-			chdir(parent);
 			begin_parse(file, mp, rec, buf, buflen);
 			if (insert_into_db(db, rec) < 0) {
 				if (mflags.verbosity)

>Release-Note:

>Audit-Trail:
From: Joerg Sonnenberger <joerg@britannica.bec.de>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: bin/51039: makemandb(8): Check for return value of chdir(2)
Date: Sun, 3 Apr 2016 14:56:09 +0200

 On Sun, Apr 03, 2016 at 12:10:00AM +0000, er.abhinav.upadhyay@gmail.com wrote:
 > makemandb makes a call to chdir(2) before parsing each page but the
 > return value of chdir is not checked. We should check the return status
 > and handle the error.

 Actually, it would be better to just keep the parent directory open and
 use fchdir.

 Joerg

From: Abhinav Upadhyay <er.abhinav.upadhyay@gmail.com>
To: NetBSD GNATS <gnats-bugs@netbsd.org>
Cc: gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Subject: Re: bin/51039: makemandb(8): Check for return value of chdir(2)
Date: Sun, 3 Apr 2016 20:33:32 +0530

 On Sun, Apr 3, 2016 at 6:30 PM, Joerg Sonnenberger
 <joerg@britannica.bec.de> wrote:
 > The following reply was made to PR bin/51039; it has been noted by GNATS.
 >
 > From: Joerg Sonnenberger <joerg@britannica.bec.de>
 > To: gnats-bugs@NetBSD.org
 > Cc:
 > Subject: Re: bin/51039: makemandb(8): Check for return value of chdir(2)
 > Date: Sun, 3 Apr 2016 14:56:09 +0200
 >
 >  On Sun, Apr 03, 2016 at 12:10:00AM +0000, er.abhinav.upadhyay@gmail.com wrote:
 >  > makemandb makes a call to chdir(2) before parsing each page but the
 >  > return value of chdir is not checked. We should check the return status
 >  > and handle the error.
 >
 >  Actually, it would be better to just keep the parent directory open and
 >  use fchdir.
 >

 You mean keeping a set of file descriptors open for each of the parent
 directories, so that instead of doing chdir each time, we use the
 already open fds for doing fchdir, and close the open fds once we are
 done doing all the parsing?

 --
 Abhinav

From: Joerg Sonnenberger <joerg@britannica.bec.de>
To: gnats-bugs@NetBSD.org
Cc: gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,
	er.abhinav.upadhyay@gmail.com
Subject: Re: bin/51039: makemandb(8): Check for return value of chdir(2)
Date: Sun, 3 Apr 2016 18:51:25 +0200

 On Sun, Apr 03, 2016 at 03:05:01PM +0000, Abhinav Upadhyay wrote:
 > The following reply was made to PR bin/51039; it has been noted by GNATS.
 > 
 > From: Abhinav Upadhyay <er.abhinav.upadhyay@gmail.com>
 > To: NetBSD GNATS <gnats-bugs@netbsd.org>
 > Cc: gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
 > Subject: Re: bin/51039: makemandb(8): Check for return value of chdir(2)
 > Date: Sun, 3 Apr 2016 20:33:32 +0530
 > 
 >  On Sun, Apr 3, 2016 at 6:30 PM, Joerg Sonnenberger
 >  <joerg@britannica.bec.de> wrote:
 >  > The following reply was made to PR bin/51039; it has been noted by GNATS.
 >  >
 >  > From: Joerg Sonnenberger <joerg@britannica.bec.de>
 >  > To: gnats-bugs@NetBSD.org
 >  > Cc:
 >  > Subject: Re: bin/51039: makemandb(8): Check for return value of chdir(2)
 >  > Date: Sun, 3 Apr 2016 14:56:09 +0200
 >  >
 >  >  On Sun, Apr 03, 2016 at 12:10:00AM +0000, er.abhinav.upadhyay@gmail.com wrote:
 >  >  > makemandb makes a call to chdir(2) before parsing each page but the
 >  >  > return value of chdir is not checked. We should check the return status
 >  >  > and handle the error.
 >  >
 >  >  Actually, it would be better to just keep the parent directory open and
 >  >  use fchdir.
 >  >
 >  
 >  You mean keeping a set of file descriptors open for each of the parent
 >  directories, so that instead of doing chdir each time, we use the
 >  already open fds for doing fchdir, and close the open fds once we are
 >  done doing all the parsing?

 Sure, the hierachy is supposed to flat (i.e. two levels under man/ at
 most), and you only need the directory open while parsing a file in that
 subtree.

 Joerg

From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/51039 CVS commit: src/usr.sbin/makemandb
Date: Tue, 12 Apr 2016 21:40:09 -0400

 Module Name:	src
 Committed By:	christos
 Date:		Wed Apr 13 01:40:09 UTC 2016

 Modified Files:
 	src/usr.sbin/makemandb: makemandb.c

 Log Message:
 PR/51039: Abhinav Upadhyay: Check for return value of chdir(2)


 To generate a diff of this commit:
 cvs rdiff -u -r1.34 -r1.35 src/usr.sbin/makemandb/makemandb.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->closed
State-Changed-By: wiz@NetBSD.org
State-Changed-When: Wed, 13 Apr 2016 07:53:17 +0000
State-Changed-Why:
Committed by christos, thanks!


>Unformatted:

Home	PR Database Search