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>
index | next in thread | raw e-mail
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);
}
help
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201008262020.o7QKKBBf075113>