d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

The unp_gc() function drops and reaquires lock between scan and

Browse Source 
collect phases.  The unp_discard() function executes
unp_externalize_fp(), which might make the socket eligible for gc-ing,
and then, later, taskqueue will close the socket.  Since unp_gc()
dropped the list lock to do the malloc, close might happen after the
mark step but before the collection step, causing collection to not
find the socket and miss one array element.

I believe that the race was there before r216158, but the stated
revision made the window much wider by postponing the close to
taskqueue sometimes.

Only process as much array elements as we find the sockets during
second phase of gc [1].  Take linkage lock and recheck the eligibility
of the socket for gc, as well as call fhold() under the linkage lock.

Reported and tested by:	jmallett
Submitted by:   jmallett [1]
Reviewed by:	rwatson, jeff (possibly)
MFC after:	1 week
This commit is contained in:
Konstantin Belousov 2011-02-01 13:33:49 +00:00
parent 03f0843bdb
commit f7780c61e7
1 changed files with 16 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
sys/kern/uipc_usrreq.c
View File

@ -2153,9 +2153,9 @@ unp_gc(__unused void *arg, int pending)
struct unp_head *heads[] = { &unp_dhead, &unp_shead, &unp_sphead,
NULL };
struct unp_head **head;
struct file **unref;
struct file *f, **unref;
struct unpcb *unp;
int i;
int i, total;
unp_taskcount++;
UNP_LIST_LOCK();
@ -2193,33 +2193,37 @@ unp_gc(__unused void *arg, int pending)
* Iterate looking for sockets which have been specifically marked
* as as unreachable and store them locally.
*/
UNP_LINK_RLOCK();
UNP_LIST_LOCK();
for (i = 0, head = heads; *head != NULL; head++)
for (total = 0, head = heads; *head != NULL; head++)
LIST_FOREACH(unp, *head, unp_link)
if (unp->unp_gcflag & UNPGC_DEAD) {
unref[i++] = unp->unp_file;
fhold(unp->unp_file);
KASSERT(unp->unp_file != NULL,
("unp_gc: Invalid unpcb."));
KASSERT(i <= unp_unreachable,
if ((unp->unp_gcflag & UNPGC_DEAD) != 0) {
f = unp->unp_file;
if (unp->unp_msgcount == 0 || f == NULL ||
f->f_count != unp->unp_msgcount)
continue;
unref[total++] = f;
fhold(f);
KASSERT(total <= unp_unreachable,
("unp_gc: incorrect unreachable count."));
}
UNP_LIST_UNLOCK();
UNP_LINK_RUNLOCK();
/*
* Now flush all sockets, free'ing rights. This will free the
* struct files associated with these sockets but leave each socket
* with one remaining ref.
*/
for (i = 0; i < unp_unreachable; i++)
for (i = 0; i < total; i++)
sorflush(unref[i]->f_data);
/*
* And finally release the sockets so they can be reclaimed.
*/
for (i = 0; i < unp_unreachable; i++)
for (i = 0; i < total; i++)
fdrop(unref[i], NULL);
unp_recycled += unp_unreachable;
unp_recycled += total;
free(unref, M_TEMP);
}