From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753269AbbJZHdX (ORCPT ); Mon, 26 Oct 2015 03:33:23 -0400 Received: from metis.ext.4.pengutronix.de ([92.198.50.35]:50649 "EHLO metis.ext.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753196AbbJZHdV (ORCPT ); Mon, 26 Oct 2015 03:33:21 -0400 Date: Mon, 26 Oct 2015 08:33:14 +0100 From: Markus Pargmann To: Oleg Nesterov Cc: Andrew Morton , Tejun Heo , nbd-general@lists.sourceforge.net, linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/1] kthread: introduce kthread_get_run() to fix __nbd_ioctl() Message-ID: <20151026073314.GD16521@pengutronix.de> References: <20151025142655.GA30961@redhat.com> <20151025142713.GA30965@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="7DO5AaGCk89r4vaK" Content-Disposition: inline In-Reply-To: <20151025142713.GA30965@redhat.com> X-Sent-From: Pengutronix Hildesheim X-URL: http://www.pengutronix.de/ X-IRC: #ptxdist @freenode X-Accept-Language: de,en X-Accept-Content-Type: text/plain X-Uptime: 08:28:47 up 36 days, 20:50, 120 users, load average: 0.23, 0.43, 1.47 User-Agent: Mutt/1.5.24 (2015-08-30) X-SA-Exim-Connect-IP: 2001:67c:670:100:1d::7 X-SA-Exim-Mail-From: mpa@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-kernel@vger.kernel.org Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --7DO5AaGCk89r4vaK Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, Oct 25, 2015 at 03:27:13PM +0100, Oleg Nesterov wrote: > It is not safe to use the task_struct returned by kthread_run(threadfn) > if threadfn() can exit before the "owner" does kthread_stop(), nothing > protects this task_struct. >=20 > So __nbd_ioctl() looks buggy; a killed nbd_thread_send() can exit, free > its task_struct, and then kthread_stop() can use the freed/reused memory. >=20 > Add the new trivial helper, kthread_get_run(). Hopefully it will have more > users, this patch changes __nbd_ioctl() as an example. Thanks. Acked-by: Markus Pargmann However I am not sure this is important for 4.3 final. This bug is present since at least 2008 (didn't look further). Best Regards, Markus >=20 > Signed-off-by: Oleg Nesterov > --- > drivers/block/nbd.c | 5 +++-- > include/linux/kthread.h | 12 ++++++++++++ > 2 files changed, 15 insertions(+), 2 deletions(-) >=20 > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c > index 93b3f99..b85e7a0 100644 > --- a/drivers/block/nbd.c > +++ b/drivers/block/nbd.c > @@ -754,8 +754,8 @@ static int __nbd_ioctl(struct block_device *bdev, str= uct nbd_device *nbd, > else > blk_queue_flush(nbd->disk->queue, 0); > =20 > - thread =3D kthread_run(nbd_thread_send, nbd, "%s", > - nbd_name(nbd)); > + thread =3D kthread_get_run(nbd_thread_send, nbd, "%s", > + nbd_name(nbd)); > if (IS_ERR(thread)) { > mutex_lock(&nbd->tx_lock); > return PTR_ERR(thread); > @@ -765,6 +765,7 @@ static int __nbd_ioctl(struct block_device *bdev, str= uct nbd_device *nbd, > error =3D nbd_thread_recv(nbd); > nbd_dev_dbg_close(nbd); > kthread_stop(thread); > + put_task_struct(thread); > =20 > mutex_lock(&nbd->tx_lock); > =20 > diff --git a/include/linux/kthread.h b/include/linux/kthread.h > index 13d5520..b0465cc 100644 > --- a/include/linux/kthread.h > +++ b/include/linux/kthread.h > @@ -37,6 +37,18 @@ struct task_struct *kthread_create_on_cpu(int (*thread= fn)(void *data), > __k; \ > }) > =20 > +/* Same as kthread_run() but also pin the task_struct */ > +#define kthread_get_run(threadfn, data, namefmt, ...) \ > +({ \ > + struct task_struct *__k \ > + =3D kthread_create(threadfn, data, namefmt, ## __VA_ARGS__); \ > + if (!IS_ERR(__k)) { \ > + get_task_struct(__k); \ > + wake_up_process(__k); \ > + } \ > + __k; \ > +}) > + > void kthread_bind(struct task_struct *k, unsigned int cpu); > void kthread_bind_mask(struct task_struct *k, const struct cpumask *mask= ); > int kthread_stop(struct task_struct *k); > --=20 > 1.5.5.1 >=20 >=20 >=20 --=20 Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | --7DO5AaGCk89r4vaK Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWLdc6AAoJEEpcgKtcEGQQLU8P/RcF4DaYf+N/HUI+SUjCuubC hVF/M8enp4Pdp5Xh7sWaum57DDRgRdMLw9cjY3wcsAHwWEJZka/dZaXQX2QpqlCQ 7Mfnj3BuvZK1WK0UvyBfUv6izDQsClhkI1GcaN+IZ1m7wTZYl2lWB3HgfJ7miPzM 23EAFJy2wUXvFl+mY4DXDXPiPNGxzwCjwwwjPDq9GJRCXdyUZs/uuOVefOyhhNsI 9uh8KbgX5+oJjXl/RsWQBGxgvv6xkGvLCdby4gq3qrjYyefZdLuEPgp2A3RCEHGQ PLcscolRplXl9WnWSRsGHOKSUt6qj/NPumsyL8+7n3ZxiU2XJohJALF2r3W6xRU3 fuyvf0LfirOQ804RdhG/mT2niqvmvCr3lR0Xy6nC/jycrZq7hmUAbmiLwMCEU0on SRvfbLvWKiTz3czI50ocKIZJBqTPzq03eXZAwsi4X/hFho4PamHQoe5F7SC0FS/s Tc2krkO5gCgdxm9uVxE3Fh1f9C7A9DYuyy9PJookvvdJGcDjZpd4S8A83i6GADAV rnZ7un9dJfAKFkgV5IkL4Rd1qmhb1SctRFfjnsHdNXzg0J4ohUzcOUL0q1zDD0It 8Fz0ZEgkXqUFFcsa30+FIF3mpHoqtO7Sv+o9XntKFwoA12TeGm1bhjFJjXTb3dHF lcUv697jN2UyUtMnTSit =KGKw -----END PGP SIGNATURE----- --7DO5AaGCk89r4vaK--