From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754013AbbI1A14 (ORCPT ); Sun, 27 Sep 2015 20:27:56 -0400 Received: from shadbolt.e.decadent.org.uk ([88.96.1.126]:36245 "EHLO shadbolt.e.decadent.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753504AbbI1A1z (ORCPT ); Sun, 27 Sep 2015 20:27:55 -0400 Message-ID: <1443400064.2517.16.camel@decadent.org.uk> Subject: Re: [PATCH 01/10] nbd: Fix timeout detection From: Ben Hutchings To: Markus Pargmann , Jens Axboe Cc: linux-kernel@vger.kernel.org, nbd-general@lists.sourceforge.net, kernel@pengutronix.de, Michal Belczyk , Hermann Lauer Date: Mon, 28 Sep 2015 01:27:44 +0100 In-Reply-To: <1439792409-28543-2-git-send-email-mpa@pengutronix.de> References: <1439792409-28543-1-git-send-email-mpa@pengutronix.de> <1439792409-28543-2-git-send-email-mpa@pengutronix.de> Content-Type: multipart/signed; micalg="pgp-sha512"; protocol="application/pgp-signature"; boundary="=-nziton9NSfgHMXN940W8" X-Mailer: Evolution 3.16.5-1 Mime-Version: 1.0 X-SA-Exim-Connect-IP: 192.168.4.247 X-SA-Exim-Mail-From: ben@decadent.org.uk X-SA-Exim-Scanned: No (on shadbolt.decadent.org.uk); SAEximRunCond expanded to false Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --=-nziton9NSfgHMXN940W8 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, 2015-08-17 at 08:20 +0200, Markus Pargmann wrote: > At the moment the nbd timeout just detects hanging tcp operations. This > is not enough to detect a hanging or bad connection as expected of a > timeout. >=20 > This patch redesigns the timeout detection to include some more cases. > The timeout is now in relation to replies from the server. If the server > does not send replies within the timeout the connection will be shut > down. >=20 > The patch adds a continous timer 'timeout_timer' that is setup in one of > two cases: > - The request list is empty and we are sending the first request out to > the server. We want to have a reply within the given timeout, > otherwise we consider the connection to be dead. > - A server response was received. This means the server is still > communicating with us. The timer is reset to the timeout value. >=20 > The timer is not stopped if the list becomes empty. It will just trigger > a timeout which will directly leave the handling routine again as the > request list is empty. >=20 > The whole patch does not use any additional explicit locking. The > list_empty() calls are safe to be used concurrently. The timer is locked > internally as we just use mod_timer and del_timer_sync(). This is crazy. The timer is locked internally but the tasks are not. So it is possible for the timeout handler to kill a task after it exited from nbd_do_it()/nbd_thread_recv(), or after it exited entirely (use-after-free). [...] > +> > task =3D READ_ONCE(nbd->task_send); > +> > if (task) > +> > > force_sig(SIGKILL, nbd->task_send); [...] And this is just... what? What is the point of using READ_ONCE() if you're going to look up nbd->task_send again? Ben. --=20 Ben Hutchings All extremists should be taken out and shot. --=-nziton9NSfgHMXN940W8 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIVAwUAVgiJgOe/yOyVhhEJAQqpig//XfyZoCf04VBvGA3C6vcj4ur05TxtqOD/ P5ESTjA2x3UjYSOx3nNMB+zdI2iPJyy7qXPfkxAAvxdggaUl5dACbOcBWxt8Oc09 8DuAo14NdUHYWry1VeftjOaCDLMwoGlC72prsYu4up4F1VJiqoD8jpTz7lCWD7ik 9lvDyHCwwJLj1JAS0JLSLlbndcV7IBP4Pqhw0m7OV4ObW19FTr6BUSGSQq0Y1JCE W2KcJYnt33Uw00UpY2WUjbr3KqldegB70r4JRZMOa5Y0yPVpH0uaclWk38plXJEr iwQUBA4nMKaxQv7/R3elr8gPgQkmURTO+oCzEDoAeI0D8m6ifln94QKsLRLxpYzZ x9Gd4NuXrC7XDZfdeejdx9fYXm4L4d7QLm4rygA65d1TVyjYqRJt8K/0EeToM+vu 7g5HjrqmOujXK8eYAQt3wdB84mrPFyR01LyY+U/uB+ST1VnXLh5zMFp2GTASoDc2 s+i9HZwKkkt1HIzwo5glotx1C8Au1NMZR23r//IYB0TprOaO6PBnX2MAckh6U3id E6tocanAENSykWkG3qZhq6uYEFy6tmnYSX4PWRyCywUM79fyyrvRxol+7c/Dbzhx /wGsHsNLpZtp4hd9efmw+VZKTip+tzaoyWXUqasdXCaCx/2VEYE5R0jLPmg74L0A /FtHSGmW6wo= =AJkk -----END PGP SIGNATURE----- --=-nziton9NSfgHMXN940W8--