Subject: Re: Time to fix a 25 year old misdesign
To: NetBSD Kernel Technical Discussion List <tech-kern@NetBSD.ORG>
From: Lennart Augustsson <lennart@augustsson.net>
List: tech-kern
Date: 10/17/2000 05:33:28
This is a multi-part message in MIME format.
--------------ED04C491D16338C88B8D5FCE
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

"Greg A. Woods" wrote:

> [ On Sunday, October 15, 2000 at 20:10:01 (+0200), Lennart Augustsson wrote: ]
> > Subject: Re: Time to fix a 25 year old misdesign
> >
> > Here's what can happen:  A process opens a device and tells the driver
> > it want's SIGIO notification.  It then forks and exits.  Since the
> > driver is not notified that the original process has closed its descriptor
> > it keeps the reference to the original process.  When this reference
> > (proc pointer or pid) gets reused a totally unsuspecting process can
> > get the signal.
> >
> > I don't see how to implement this right unless the driver gets notified
> > on each close so it can drop the reference to the process.
>
> This would be trivial to implement with a simple extension to the driver
> API, such as a "dev_sigio_set()" call that registers a process ID for
> the driver interrupt routine to trigger SIGIO when the correct
> conditions have been met at the hardware level.  The upper layer would
> simply de-register the PID upon close(2), perhaps with a corresponding
> "dev_sigio_unset()" call.

But now you are solving the SIGIO problem by adding two special purpose
driver methods.  I also want to add methods too, but they have other uses as well.


> > Am I mistaken, or has SIGIO been broken since its inception?
> > (I could be mistaken, I've had enough coffee today. :)
>
> I haven't had enough coffee yet today, but I think you're partly right,
> though I don't think there's currently any possibility of the scenario
> you described happening in currently implemented drivers (at least not
> so far as I can find with grep).

I don't think I'm just partly right, I think I'm right. :)
I'll include two programs below.  Run the first one, it will output a pid.
Then run the second one (in another window).  When the second
program tells you that it has managed to get the pid you gave it on the
command line then hit return for the first program.  See how the second
program reports a SIGIO.  If you want it to be a little more spectacular
you can run the second program as root and the first as a normal user.

As you can see, a user program can send a SIGIO to a process owned
by root without the root process expecting it.

My sample program uses sockets, but you can use some other device
that supports SIGIO as well.


> It seems as though at least most drivers can only send SIGIO to one
> process at a time (since they store the process to signal in a single
> "struct proc *" field in their "struct softc"), and so long as those
> drivers also all require exclusive access there's nothing literally
> broken -- it's just that the design is perhaps an accident waiting to
> happen because it depends upon a side-effect in the open() routine....

But since a process can register to get a SIGIO and then die it is broken.
And the brokenness comes from the fact that since there are multiple
references to the file descriptor the death of the process is not the last
close so the driver is not notified.


--

        -- Lennart



--------------ED04C491D16338C88B8D5FCE
Content-Type: text/plain; charset=us-ascii;
 name="async.c"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="async.c"

#include <sys/ioctl.h>
#include <stdio.h>
#include <signal.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <signal.h>
#include <err.h>
#include <unistd.h>
#include <netinet/in.h>

void
io()
{
	printf("pid %d got SIGIO\n", getpid());
	exit(0);
}


int
main(int argc, char **argv)
{
	int one = 1;
	int s, pid, s0;
	char buf[1];
	int port = htons(2003);
	struct sockaddr_in sa;

	s0 = socket(PF_INET, SOCK_DGRAM, IPPROTO_UDP);
	if (s0 < 0)
		err(1, "socket");

	if (fork()) {
		s = socket(PF_INET, SOCK_DGRAM, IPPROTO_UDP);
		if (s < 0)
			err(1, "socket 1");

		sa.sin_port = port;
		sa.sin_family = AF_INET;
		sa.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
		sleep(1);
		printf("hit return to send\n");
		read(0, buf, 1);
		printf("send\n");
		if (sendto(s, "xxxx", 4, 0, 
			   (struct sockaddr *)&sa, sizeof sa) < 0)
			err(1, "sendto");

		sigsuspend(0);
	} else {
		if (fork())
			_exit(0);

		if (signal(SIGIO, io) < 0)
			err(1, "signal");
		pid = getpid();
		if (ioctl(s0, SIOCSPGRP, &pid) < 0)
			err(1, "ioctl s");
		if (ioctl(s0, FIOASYNC, &one) < 0)
			err(1, "ioctl a");

		sa.sin_port = port;
		sa.sin_family = AF_INET;
		sa.sin_addr.s_addr = htonl(INADDR_ANY);
		if (bind(s0, (struct sockaddr *)&sa, sizeof sa) < 0)
			err(1, "bind");
		printf("sending signal to pid %d\n", getpid());
		exit(0);
		/*sigsuspend(0);*/
	}

	exit(0);
}

--------------ED04C491D16338C88B8D5FCE
Content-Type: text/plain; charset=us-ascii;
 name="procpid.c"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="procpid.c"

#include <stdio.h>
#include <stdlib.h>
#include <sys/types.h>
#include <unistd.h>
#include <signal.h>
#include <err.h>

void
io()
{
	printf("got SIGIO\n");
	exit(0);
}

#define MAX 100000

int
main(int argc, char **argv)
{
	int pid = atoi(argv[1]);
	int start = getpid();
	int p, i;

	for (i = 0; i < MAX; i++) {
		p = fork();
		if (p < 0) {
			/*err(1, "fork");*/
			usleep(10000);
			continue;
		}
		if (p)
			_exit(0);
		p = getpid();
		/*printf("%d\n", p);*/
		if (p == pid)
			break;
		if (p == start)
			errx(1, "wrapped around\n");
	}
	if (i >= MAX)
		exit(1);
	printf("got pid %d\n", pid);
	signal(SIGIO, io);
	sigsuspend(0);
	exit(0);
}


--------------ED04C491D16338C88B8D5FCE--