Another problem caused by the knlist_cleardel() potentially dropping

PIPE_MTX().

Since the pipe_present is cleared before (potentially) sleeping, the
second thread may enter the pipeclose() for the reciprocal pipe end.
The test at the end of the pipeclose() for the pipe_present == 0 would
succeed, allowing the second thread to free the pipe memory. First
threads then accesses the freed memory after being woken up.

Properly track the closing state of the pipe in the pipe_present.
Introduce the intermediate state that marks the pipe as mostly
dismantled but might be sleeping waiting for the knote list to be
cleared. Free the pipe pair memory only when both ends pass that point.

Debugging help and tested by:	pho
Discussed with:	jmg
MFC after:	2 weeks
This commit is contained in:
Konstantin Belousov 2008-05-23 11:14:03 +00:00
parent e2e1693f15
commit 741b6cf8a5
2 changed files with 29 additions and 11 deletions

View File

@ -265,8 +265,8 @@ pipe_zone_ctor(void *mem, int size, void *arg, int flags)
* one at a time. When both are free'd, then the whole pair
* is released.
*/
rpipe->pipe_present = 1;
wpipe->pipe_present = 1;
rpipe->pipe_present = PIPE_ACTIVE;
wpipe->pipe_present = PIPE_ACTIVE;
/*
* Eventually, the MAC Framework may initialize the label
@ -975,7 +975,8 @@ pipe_write(fp, uio, active_cred, flags, td)
/*
* detect loss of pipe read side, issue SIGPIPE if lost.
*/
if ((!wpipe->pipe_present) || (wpipe->pipe_state & PIPE_EOF)) {
if (wpipe->pipe_present != PIPE_ACTIVE ||
(wpipe->pipe_state & PIPE_EOF)) {
pipeunlock(wpipe);
PIPE_UNLOCK(rpipe);
return (EPIPE);
@ -1346,13 +1347,14 @@ pipe_poll(fp, events, active_cred, td)
revents |= events & (POLLIN | POLLRDNORM);
if (events & (POLLOUT | POLLWRNORM))
if (!wpipe->pipe_present || (wpipe->pipe_state & PIPE_EOF) ||
if (wpipe->pipe_present != PIPE_ACTIVE ||
(wpipe->pipe_state & PIPE_EOF) ||
(((wpipe->pipe_state & PIPE_DIRECTW) == 0) &&
(wpipe->pipe_buffer.size - wpipe->pipe_buffer.cnt) >= PIPE_BUF))
revents |= events & (POLLOUT | POLLWRNORM);
if ((rpipe->pipe_state & PIPE_EOF) ||
(!wpipe->pipe_present) ||
wpipe->pipe_present != PIPE_ACTIVE ||
(wpipe->pipe_state & PIPE_EOF))
revents |= POLLHUP;
@ -1493,7 +1495,7 @@ pipeclose(cpipe)
* Disconnect from peer, if any.
*/
ppipe = cpipe->pipe_peer;
if (ppipe->pipe_present != 0) {
if (ppipe->pipe_present == PIPE_ACTIVE) {
pipeselwakeup(ppipe);
ppipe->pipe_state |= PIPE_EOF;
@ -1510,16 +1512,23 @@ pipeclose(cpipe)
PIPE_UNLOCK(cpipe);
pipe_free_kmem(cpipe);
PIPE_LOCK(cpipe);
cpipe->pipe_present = 0;
cpipe->pipe_present = PIPE_CLOSING;
pipeunlock(cpipe);
/*
* knlist_clear() may sleep dropping the PIPE_MTX. Set the
* PIPE_FINALIZED, that allows other end to free the
* pipe_pair, only after the knotes are completely dismantled.
*/
knlist_clear(&cpipe->pipe_sel.si_note, 1);
cpipe->pipe_present = PIPE_FINALIZED;
knlist_destroy(&cpipe->pipe_sel.si_note);
/*
* If both endpoints are now closed, release the memory for the
* pipe pair. If not, unlock.
*/
if (ppipe->pipe_present == 0) {
if (ppipe->pipe_present == PIPE_FINALIZED) {
PIPE_UNLOCK(cpipe);
#ifdef MAC
mac_pipe_destroy(pp);
@ -1543,7 +1552,7 @@ pipe_kqfilter(struct file *fp, struct knote *kn)
break;
case EVFILT_WRITE:
kn->kn_fop = &pipe_wfiltops;
if (!cpipe->pipe_peer->pipe_present) {
if (cpipe->pipe_peer->pipe_present != PIPE_ACTIVE) {
/* other end of pipe has been closed */
PIPE_UNLOCK(cpipe);
return (EPIPE);
@ -1586,7 +1595,8 @@ filt_piperead(struct knote *kn, long hint)
kn->kn_data = rpipe->pipe_map.cnt;
if ((rpipe->pipe_state & PIPE_EOF) ||
(!wpipe->pipe_present) || (wpipe->pipe_state & PIPE_EOF)) {
wpipe->pipe_present != PIPE_ACTIVE ||
(wpipe->pipe_state & PIPE_EOF)) {
kn->kn_flags |= EV_EOF;
PIPE_UNLOCK(rpipe);
return (1);
@ -1604,7 +1614,8 @@ filt_pipewrite(struct knote *kn, long hint)
struct pipe *wpipe = rpipe->pipe_peer;
PIPE_LOCK(rpipe);
if ((!wpipe->pipe_present) || (wpipe->pipe_state & PIPE_EOF)) {
if (wpipe->pipe_present != PIPE_ACTIVE ||
(wpipe->pipe_state & PIPE_EOF)) {
kn->kn_data = 0;
kn->kn_flags |= EV_EOF;
PIPE_UNLOCK(rpipe);

View File

@ -114,6 +114,13 @@ struct pipe {
int pipe_present; /* still present? */
};
/*
* Values for the pipe_present.
*/
#define PIPE_ACTIVE 1
#define PIPE_CLOSING 2
#define PIPE_FINALIZED 3
/*
* Container structure to hold the two pipe endpoints, mutex, and label
* pointer.