Date: Sun, 9 Oct 2016 22:07:01 -0700 From: Julian Elischer <julian@freebsd.org> To: Oliver Pinter <oliver.pinter@hardenedbsd.org> Cc: FreeBSD Stable <freebsd-stable@freebsd.org>, freebsd <freebsd-hackers@freebsd.org> Subject: Re: fix for use-after-free problem in 10.x Message-ID: <0f543bb5-468e-e559-1bd8-8e2cf3f8bbc3@freebsd.org> In-Reply-To: <CAPQ4ffv8MzSUwvSDL=WG300tP3ng0YdSyTEkWrwXxb%2BMmw58gQ@mail.gmail.com> References: <7b732876-8cc3-a638-7ff1-e664060d4907@freebsd.org> <CAPQ4ffv8MzSUwvSDL=WG300tP3ng0YdSyTEkWrwXxb%2BMmw58gQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 8/10/2016 5:36 AM, Oliver Pinter wrote: > On 10/5/16, Julian Elischer <julian@freebsd.org> 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? > Yes, please commit them. This patch fixes the ZFS + GELI + INVARIANTS > problem for us. > There is the FreeBSD PR about the issue: > https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=209580 I committed a slightly better version to stable/10 should I ask for a merge to releng/10.3? > >> here's the fix in diff form: >> >> >> [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")); >> >> _______________________________________________ >> freebsd-hackers@freebsd.org mailing list >> https://lists.freebsd.org/mailman/listinfo/freebsd-hackers >> To unsubscribe, send any mail to "freebsd-hackers-unsubscribe@freebsd.org" >>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?0f543bb5-468e-e559-1bd8-8e2cf3f8bbc3>