Re: ipv6: tunnel: hang when destroying ipv6 tunnel

From: Tetsuo Handa
Date: Fri Apr 06 2012 - 14:48:26 EST


Tetsuo Handa wrote:
> Most suspicious change is net/9p/client.c because it is changing handling of
> ERESTARTSYS case.
>
> --- linux-3.3.1/net/9p/client.c
> +++ linux-next/net/9p/client.c
> @@ -740,10 +740,18 @@
> c->status = Disconnected;
> goto reterr;
> }
> +again:
> /* Wait for the response */
> err = wait_event_interruptible(*req->wq,
> req->status >= REQ_STATUS_RCVD);
>
> + if ((err == -ERESTARTSYS) && (c->status == Connected)
> + && (type == P9_TFLUSH)) {
> + sigpending = 1;
> + clear_thread_flag(TIF_SIGPENDING);
> + goto again;
> + }
> +

I think this loop is bad with regard to response to SIGKILL.
If wait_event_interruptible() was interrupted by SIGKILL, it will
spin until req->status >= REQ_STATUS_RCVD becomes true.
Rather,

if ((c->status == Connected) && (type == P9_TFLUSH))
err = wait_event_killable(*req->wq,
req->status >= REQ_STATUS_RCVD);
else
err = wait_event_interruptible(*req->wq,
req->status >= REQ_STATUS_RCVD);

would be safer.



> error:
> /*
> * Fid is not valid even after a failed clunk
> + * If interrupted, retry once then give up and
> + * leak fid until umount.
> */
> - p9_fid_destroy(fid);
> + if (err == -ERESTARTSYS) {
> + if (retries++ == 0)
> + goto again;

I think it is possible that the process is interrupted again upon retrying.
I suspect the handling of err == -ERESTARTSYS case when retries != 0.
It is returning without calling p9_fid_destroy(), which will be
unexpected behaviour for the various callers.

> + } else
> + p9_fid_destroy(fid);
> return err;
> }
> EXPORT_SYMBOL(p9_client_clunk);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/