Date: Thu, 26 Aug 2010 20:20:11 GMT From: Kostik Belousov <kostikbel@gmail.com> To: freebsd-bugs@FreeBSD.org Subject: Re: kern/143029: [libc] poll(2) can return too large a number Message-ID: <201008262020.o7QKKBBf075113@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/143029; it has been noted by GNATS. From: Kostik Belousov <kostikbel@gmail.com> To: bug-followup@FreeBSD.org, jplevyak@apache.org Cc: Subject: Re: kern/143029: [libc] poll(2) can return too large a number Date: Thu, 26 Aug 2010 23:16:33 +0300 I believe the issue happens because for some file descriptors, select code registers two selfd structures. E.g., for socket, when specified POLLIN|POLLOUT in events, you would have one selfd registered for receiving socket buffer, and one for sending. Now, if both events are not ready to fire at the time of the initial scan, but are simultaneously ready after the sleep, pollrescan() would iterate over the pollfd struct twice. It is not harmful by itself, but pollfd gets accounted twice. The crusial part is the simultaneous firing of read and write ready events after the sleep, due to /* If the selinfo wasn't cleared the event didn't fire. */ if (si != NULL) continue; code in the pollrescan() loop. In the simple case when only one event fired, no double-accounting happen. Patch below should fix it. Could you, please, confirm ? diff --git a/sys/kern/sys_generic.c b/sys/kern/sys_generic.c index a1a7086..2b05faf 100644 --- a/sys/kern/sys_generic.c +++ b/sys/kern/sys_generic.c @@ -76,7 +76,8 @@ static MALLOC_DEFINE(M_IOCTLOPS, "ioctlops", "ioctl data buffer"); static MALLOC_DEFINE(M_SELECT, "select", "select() buffer"); MALLOC_DEFINE(M_IOV, "iov", "large iov's"); -static int pollout(struct pollfd *, struct pollfd *, u_int); +static int pollout(struct thread *, struct pollfd *, struct pollfd *, + u_int); static int pollscan(struct thread *, struct pollfd *, u_int); static int pollrescan(struct thread *); static int selscan(struct thread *, fd_mask **, fd_mask **, int); @@ -1207,7 +1208,7 @@ done: if (error == EWOULDBLOCK) error = 0; if (error == 0) { - error = pollout(bits, uap->fds, nfds); + error = pollout(td, bits, uap->fds, nfds); if (error) goto out; } @@ -1262,22 +1263,27 @@ pollrescan(struct thread *td) static int -pollout(fds, ufds, nfd) +pollout(td, fds, ufds, nfd) + struct thread *td; struct pollfd *fds; struct pollfd *ufds; u_int nfd; { int error = 0; u_int i = 0; + u_int n = 0; for (i = 0; i < nfd; i++) { error = copyout(&fds->revents, &ufds->revents, sizeof(ufds->revents)); if (error) return (error); + if (fds->revents != 0) + n++; fds++; ufds++; } + td->td_retval[0] = n; return (0); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201008262020.o7QKKBBf075113>