Skip site navigation (1)Skip section navigation (2) 
Date:      Sat, 10 Jan 2009 17:39:07 GMT
From:      Ivan Shcheklein <shcheklein@gmail.com>
To:        freebsd-gnats-submit@FreeBSD.org
Subject:   kern/130348: [socket] accept() prematurely allocates an inheritable descriptor
Message-ID:  <200901101739.n0AHd7t3049481@www.freebsd.org>
Resent-Message-ID: <200901101740.n0AHe1lw030999@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help 

>Number:         130348
>Category:       kern
>Synopsis:       [socket] accept() prematurely allocates an inheritable descriptor
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Sat Jan 10 17:40:01 UTC 2009
>Closed-Date:
>Last-Modified:
>Originator:     Ivan Shcheklein
>Release:        FreeBSD 7.1
>Organization:
ISP RAS
>Environment:
FreeBSD freebsd2.localdomain 7.1-RELEASE FreeBSD 7.1-RELEASE #0: Fri Jan  9 23:36:55 MSK 2009     modis@freebsd2.localdomain:/usr/obj/usr/src/sys/GENERIC  i386

>Description:
kern_accept() allocates a file descriptor before it is blocked until a connection is present. This descriptor could be unexpectedly inherited if the process calls exec() in a different thread.

It means that the child process may obtain a connected descriptor it doesn't know anything about. Moreover, parent process also doesn't expect that there are references on this descriptor in the system.

Seems this behaviour appeared first in 1.186 revision:
http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/kern/uipc_syscalls.c#rev1.186:

 "Reorganize the optimistic concurrency behavior in accept1() to
  always allocate a file descriptor with falloc() so that if we do
  find a socket, we don't have to encounter the "Oh, there wasn't
  a socket" race that can occur if falloc() sleeps in the current
  code, which broke inbound accept() ordering, not to mention
  requiring backing out socket state changes in a way that raced
  with the protocol level.  We may want to add a lockless read of
  the queue state if polling of empty queues proves to be important
  to optimize."
>How-To-Repeat:
1. Build (cc -Wall server.c -o server) the following code:

#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>
#include <netdb.h>
#include <sys/socket.h>
#include <sys/un.h>
#include <errno.h>
#include <string.h>
#include <netinet/in.h>

int main()
{
	int fd, error = 0;
	struct sockaddr_in in;
	struct hostent *hp;

	if ((fd = socket(AF_INET, SOCK_STREAM, 0)) == -1) {
		perror("socket");
		error = -1;
		goto done;	
	}
	if((hp = gethostbyname("0.0.0.0")) == NULL) {
		perror("gethostbyname");
		error = -2;
		goto done;	
	}
		
	memset(&in, 0, sizeof(in));
	in.sin_family = AF_INET;
	in.sin_port = htons(5050);
	memcpy(&in.sin_addr, hp->h_addr, hp->h_length);

	if (bind(fd, (struct sockaddr *)&in, sizeof(in)) < 0) {
		perror("bind");
		error = -3;
		goto done;	
	}
	if (listen(fd, 10) < 0) {
		perror("listen");
		error = -4;
		goto done;	
	}
	if (accept(fd,0,0) < 0) {
		perror("accept");
		error = -5;
	}

done:	close(fd);
	return error;
}

2. Run "lsof | grep server". You will see a number of file descriptors and among them should be something like this:

server    1076  root    3u    IPv4 0xc50ec910        0t0     TCP *:mmcc (LISTEN)
server    1076  root    4                                        0xc584b090 file struct, ty=0, op=0xc0979ec0

The first one (3u) is the descriptor we call accept() on. The second one (4) is a file struct which is allocated by falloc() in kern_accept(). It is inheritable. Therefore child may obtain a connection it doesn't expect.
>Fix:


>Release-Note:
>Audit-Trail:
>Unformatted:

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200901101739.n0AHd7t3049481>