Re: SE GC83 GPRS card: pppd problem

Jachym Holecek <[email protected]>
Newsgroups gmane.os.netbsd.ports.hpcarm,gmane.os.netbsd.current
Message-ID <[email protected]>
> On Jan 30,  5:16pm, [email protected] (Jachym Holecek) wrote:
> -- Subject: Re: SE GC83 GPRS card: pppd problem
> | 
> | I did the same last night (but offline) -- the problem was in main.c
> | (in case you really mean sys-bsd.c, I can take a look later in the
> | evening)
> 
> I really meant sys-bsd.c ...

dist/pppd/pppd/main.c contains unhandled corner case, the patch
below fixed the problem I've reproduced with Alex's setup. The
second occupy_stdfds() call is key.

	-- Jachym

--- main.c	2006-01-30 17:52:56.000000000 +0100
+++ /home/jh/main.c	2006-01-30 17:54:17.000000000 +0100
@@ -1525,6 +1525,23 @@
 }
 
 /*
+ * occupy_stdfds - make sure fds 0, 1, 2 are occupied by pointing them
+ * to the /dev/null device.
+ */
+static void
+occupy_stdfds(void)
+{
+	int fd;
+
+	while ((fd = dup(fd_devnull)) >= 0) {
+		if (fd > 2) {
+			close(fd);
+			break;
+		}
+	}
+}
+
+/*
  * safe_fork - Create a child process.  The child closes all the
  * file descriptors that we don't want to leak to a script.
  * The parent waits for the child to do this before returning.
@@ -1535,16 +1552,11 @@
 safe_fork(int infd, int outfd, int errfd)
 {
 	pid_t pid;
-	int fd, pipefd[2];
+	int pipefd[2];
 	char buf[1];
 
-	/* make sure fds 0, 1, 2 are occupied (probably not necessary) */
-	while ((fd = dup(fd_devnull)) >= 0) {
-		if (fd > 2) {
-			close(fd);
-			break;
-		}
-	}
+	/* make sure the pipes will be above fd 2 */
+	occupy_stdfds();
 
 	if (pipe(pipefd) == -1)
 		pipefd[0] = pipefd[1] = -1;
@@ -1568,6 +1580,9 @@
 	tdb_close(pppdb);
 #endif
 
+	/* we've closed some descriptors, need to ensure again */
+	occupy_stdfds();
+
 	/* make sure infd, outfd and errfd won't get tromped on below */
 	if (infd == 1 || infd == 2)
 		infd = dup(infd);
@@ -1577,28 +1592,31 @@
 		errfd = dup(errfd);
 
 	/* dup the in, out, err fds to 0, 1, 2 */
-	if (infd != 0)
+	if (infd != 0) {
 		dup2(infd, 0);
-	if (outfd != 1)
+		close(infd);
+	}
+	if (outfd != 1) {
 		dup2(outfd, 1);
-	if (errfd != 2)
+		close(outfd);
+	}
+	if (errfd != 2) {
 		dup2(errfd, 2);
+		close(errfd);
+	}
 
 	closelog();
 	if (log_to_fd > 2)
 		close(log_to_fd);
-	if (the_channel->close)
+	if (the_channel->close) {
 		(*the_channel->close)();
-	else
-		close(devfd);	/* some plugins don't have a close function */
+		notice("teh_channel close");
+	} else {
+		if (devfd > 0)
+			close(devfd);	/* some plugins don't have a close() */
+	}
 	close(fd_ppp);
 	close(fd_devnull);
-	if (infd != 0)
-		close(infd);
-	if (outfd != 1)
-		close(outfd);
-	if (errfd != 2)
-		close(errfd);
 
 	notify(fork_notifier, 0);
 	close(pipefd[0]);
@@ -1631,6 +1649,8 @@
     ++conn_running;
     pid = safe_fork(in, out, errfd);
 
+    if (pid == 0)
+	if (log_to_fd
     if (pid != 0 && log_to_fd < 0)
 	close(errfd);
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.