Date: Wed, 5 Oct 2016 16:59:05 -0700 From: Julian Elischer <julian@freebsd.org> To: FreeBSD Stable <freebsd-stable@FreeBSD.org>, freebsd <freebsd-hackers@freebsd.org> Subject: Re: fix for use-after-free problem in 10.x [review please]. Message-ID: <6eaa1131-e268-bfb4-9203-a93eaf296f6c@freebsd.org> In-Reply-To: <7b732876-8cc3-a638-7ff1-e664060d4907@freebsd.org> References: <7b732876-8cc3-a638-7ff1-e664060d4907@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Please review..
https://reviews.freebsd.org/D8160
Direct fix for stable/10 as bug is not present in 11+ in this form.
Julian
On 4/10/2016 8:06 PM, Julian Elischer wrote:
> In 11 and 12 the taskqueue code has been rewritten in this area but
> under 10 this bug still occurs.
>
> On our appliances this bug stops the system from mounting the ZFS
> root, so it is quite severe.
> Basically while the thread is sleeping during the ZFS mount of root
> (in the while loop), another thread can free the 'task' item it is
> checking in that while loop and it can be reused or filled with
> 'deadcode' etc., with the waiting code unaware of the change.. The
> fix is to refetch the item at the end of the queue each time around
> the loop.
> I don't really want to do the bigger change of MFCing the change in
> 11, as it is more extensive, though if someone else does, that's ok
> by me. (If it's ABI compatible)
>
> Any comments or suggestions?
>
> here's the fix in diff form:
A slightly better fix is at
https://reviews.freebsd.org/D8160
>
>
> [robot@porridge /usr/src]$ p4 diff -du ...
> ---
> //depot/pbranches/jelischer/FreeBSD-PZ/10.3/sys/kern/subr_taskqueue.c
> 2016-09-27 09:14:59.000000000 -0700
> +++ /usr/src/sys/kern/subr_taskqueue.c 2016-09-27
> 09:14:59.000000000 -0700
> @@ -441,9 +441,10 @@
>
> TQ_LOCK(queue);
> task = STAILQ_LAST(&queue->tq_queue, task, ta_link);
> - if (task != NULL)
> - while (task->ta_pending != 0)
> - TQ_SLEEP(queue, task, &queue->tq_mutex,
> PWAIT, "-", 0);
> + while (task != NULL && task->ta_pending != 0) {
> + TQ_SLEEP(queue, task, &queue->tq_mutex, PWAIT, "-", 0);
> + task = STAILQ_LAST(&queue->tq_queue, task, ta_link);
> + }
> taskqueue_drain_running(queue);
> KASSERT(STAILQ_EMPTY(&queue->tq_queue),
> ("taskqueue queue is not empty after draining"));
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6eaa1131-e268-bfb4-9203-a93eaf296f6c>