fix for use-after-free problem in 10.x

[Pintér, Olivér]

On Mon, Oct 10, 2016 at 7:07 AM, Julian Elischer <julian at freebsd.org> wrote:

> On 8/10/2016 5:36 AM, Oliver Pinter wrote:
>
>> On 10/5/16, Julian Elischer <julian at 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?


Yes, it would be really nice! Thanks your effort!



>
>
>
>
>
>> here's the fix in diff form:
>>>
>>>
>>> [robot at porridge /usr/src]$ p4 diff -du ...
>>> --- //depot/pbranches/jelischer/FreeBSD-PZ/10.3/sys/kern/subr_ta
>>> skqueue.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 at freebsd.org mailing list
>>> https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
>>> To unsubscribe, send any mail to "freebsd-hackers-unsubscribe at f
>>> reebsd.org"
>>>
>>>
> _______________________________________________
> freebsd-stable at freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-stable
> To unsubscribe, send any mail to "freebsd-stable-unsubscribe at freebsd.org"
>