soo_close() vs. filt_soread()

[Konstantin Belousov]

On Tue, Oct 29, 2013 at 11:35:04AM +0100, Sebastian Huber wrote:
> Hello,
> 
> I port currently the FreeBSD network stack to a real-time operating system. 
> The problem described below probably does not happen in a real FreeBSD kernel. 
>   I have the following test case:
> 
> static void
> test_kqueue_close(test_context *ctx)
> {
> 	/* The cfd is some socket with a connected TCP stream */
> 	int cfd = ctx->cfd;
> 	int kq;
> 	struct kevent change;
> 	struct kevent event;
> 	const struct timespec *timeout = NULL;
> 	int rv;
> 	ssize_t n;
> 
> 	puts("test kqueue close");
> 
> 	assert(ctx->cfd >= 0);
> 
> 	kq = kqueue();
> 	assert(kq >= 0);
> 
> 	EV_SET(&change, cfd, EVFILT_READ, EV_ADD | EV_ENABLE, 0, 0,
> 	    TEST_UDATA);
> 
> 	rv = kevent(kq, &change, 1, NULL, 0, timeout);
> 	assert(rv == 0);
> 
> 	set_non_blocking(cfd, 1);
> 
> 	do {
> 		errno = 0;
> 		n = read(cfd, &ctx->buf[0], sizeof(ctx->buf));
> 		if (n == -1) {
> 			assert(errno = EAGAIN);
> 		}
> 	} while (n > 0);
> 
> 
> 	/* This tells some background entity that we want to close cfd once kevent 
> blocks */
> 	send_events(ctx, EVENT_CLOSE);
> 
> 	assert(ctx->cfd >= 0);
> 
> 	rv = kevent(kq, NULL, 0, &event, 1, timeout);
> 	assert(rv == 1);
> 	assert(event.ident == cfd);
> 	assert(event.filter == EVFILT_READ);
> 	assert(event.flags == 0);
> 	assert(event.fflags == 0);
> 	assert(event.data == 0);
> 	assert(event.udata == TEST_UDATA);
> 
> 	assert(ctx->cfd == -1);
> 
> 	rv = close(kq);
> 	assert(rv == 0);
> }
> 
> This test registers a read event and then blocks for this event.  Once the 
> current thread blocked, someone will delete the corresponding socket.  I have 
> now a NULL pointer access.  The socket close looks like this:
> 
> /*
>   * API socket close on file pointer.  We call soclose() to close the socket
>   * (including initiating closing protocols).  soclose() will sorele() the
>   * file reference but the actual socket will not go away until the socket's
>   * ref count hits 0.
>   */
> /* ARGSUSED */
> int
> soo_close(struct file *fp, struct thread *td)
> {
> 	int error = 0;
> 	struct socket *so;
> 
> 	so = fp->f_data;
> 	fp->f_ops = &badfileops;
> 	fp->f_data = NULL;
> 
> 	if (so)
> 		error = soclose(so);
> 	return (error);
> }
> 
> Please note that fp->f_data is set to NULL.
> 
> The close operation will end up in:
> 
> /*ARGSUSED*/
> static int
> filt_soread(struct knote *kn, long hint)
> {
> 	struct socket *so;
> 
> 	so = kn->kn_fp->f_data;
> 	SOCKBUF_LOCK_ASSERT(&so->so_rcv);
> 
> 	kn->kn_data = so->so_rcv.sb_cc - so->so_rcv.sb_ctl;
> 	if (so->so_rcv.sb_state & SBS_CANTRCVMORE) {
> 		kn->kn_flags |= EV_EOF;
> 		kn->kn_fflags = so->so_error;
> 		return (1);
> 	} else if (so->so_error)	/* temporary udp error */
> 		return (1);
> 	else if (kn->kn_sfflags & NOTE_LOWAT)
> 		return (kn->kn_data >= kn->kn_sdata);
> 	else
> 		return (so->so_rcv.sb_cc >= so->so_rcv.sb_lowat);
> }
> 
> Here so == NULL, due to soo_close().
> 
> Breakpoint 11, filt_soread (kn=0x46ff74, hint=0) at 
> freebsd/sys/kern/uipc_socket.c:3147
> 3147            so = kn->kn_fp->f_data;
> (gdb) p so
> $4 = (struct socket *) 0x0
> (gdb) p kn->kn_ptr.p_fp
> $5 = (struct file *) 0x32cec8
> (gdb) where
> #0  filt_soread (kn=0x46ff74, hint=0) at freebsd/sys/kern/uipc_socket.c:3147
> #1  0x0010506e in knote (list=0x3f1190, hint=0, lockflags=1) at 
> freebsd/sys/kern/kern_event.c:1957
> #2  0x00191a60 in sowakeup (so=0x3f1134, sb=0x3f1188) at 
> freebsd/sys/kern/uipc_sockbuf.c:191
> #3  0x00127afe in soisdisconnecting (so=0x3f1134) at 
> freebsd/sys/kern/uipc_socket.c:3341
> #4  0x00153cca in tcp_disconnect (tp=0x3f75ac) at 
> freebsd/sys/netinet/tcp_usrreq.c:1508
> #5  0x00152b30 in tcp_usr_disconnect (so=0x3f1134) at 
> freebsd/sys/netinet/tcp_usrreq.c:556
> #6  0x00124754 in sodisconnect (so=0x3f1134) at freebsd/sys/kern/uipc_socket.c:816
> #7  0x00124384 in soclose (so=0x3f1134) at freebsd/sys/kern/uipc_socket.c:664
> #8  0x00128886 in soo_close (fp=0x32cec8, td=0x0) at 
> freebsd/sys/kern/sys_socket.c:452
> 
> Is this an illegal kevent() use case?  Are there some means that prevent this 
> sequence in a real FreeBSD kernel?

When file descriptor gets closed, kevent subsystem is notified first
and clears knotes related to the filedescriptor (important, not the
file) being closed. You could see this in the closefp() function calling
knote_fdclose(). Since files are referenced and destroyed only when the
last reference vanishes, and filedescriptor holds a reference on the
file, file (which is socket in your case) cannot go away until knotes
are expunged.

There are some corner cases, when knotes are really attached to objects
with different life-cycle, like vnodes, but I believe the description
above should be accurate enough for sockets.

If there is a problem in the FreeBSD code and you could produce a test
case, do not hesitate to send it to me.

Thank you.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 834 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/freebsd-hackers/attachments/20131030/566082ba/attachment.sig>