[Bug 262633] Missing mtx protecting variable prison->pr_prison_racct

From: <bugzilla-noreply_at_freebsd.org>
Date: Thu, 17 Mar 2022 21:04:49 UTC
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=262633

            Bug ID: 262633
           Summary: Missing mtx protecting variable
                    prison->pr_prison_racct
           Product: Base System
           Version: CURRENT
          Hardware: Any
                OS: Any
            Status: New
          Severity: Affects Only Me
          Priority: ---
         Component: kern
          Assignee: bugs@FreeBSD.org
          Reporter: firk@cantconnect.ru

jail.h says:

>         struct prison_racct *pr_prison_racct; /* (c) racct jail proxy */

> *   (c) set only during creation before the structure is shared, no mutex
> *       required to read

This is not true, there is kern_jail.c/jail_racct_modify() function which can
alter this field in runtime. Renaming jails is very rare case, and it usually
is not available to any non-toplevel-root processes, but it is still possible.

While all the above was just a theory (but I don't know all possible practical
consequences), there is at least one case that may lead to panic. Let's see:

> jail_racct_modify() {
>     // ...
>     pr->pr_prison_racct = NULL;
>     prison_racct_attach(pr) {
>         sx_assert(&allprison_lock, SA_XLOCKED);
>         prr = prison_racct_find_locked(pr->pr_name) {
>             // ...
>             LIST_FOREACH(prr, &allprison_racct, prr_next) {
>                 if (strcmp(name, prr->prr_name) != 0) continue;
>                 prison_racct_hold(prr);
>                 return (prr);
>             }
>             prr = malloc(sizeof(*prr), M_PRISON_RACCT, M_ZERO | M_WAITOK);
>             // ...
>         }
>         pr->pr_prison_racct = prr;
>     }

This LIST_FOREACH may be time-consuming if we have many prison names already
registered. Also, there is malloc(M_WAITOK) after it. All this time
pr_prison_racct is NULL, while, for example, racct_add_cred_locked() walks
through pr->pr_prison_racct->prr_racct without any prison-related locks and
may dereference this NULL.

I'll try to make test program for this later.

-- 
You are receiving this mail because:
You are the assignee for the bug.