tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

unsafe ->p_cwdi access in mount_checkdirs



In mount_checkdirs you can find a loop:

mutex_enter(proc_lock);
PROCLIST_FOREACH(p, &allproc) {
        if ((cwdi = p->p_cwdi) == NULL)
                continue;
        if (cwdi->cwdi_cdir != olddp &&
            cwdi->cwdi_rdir != olddp)
                continue;
        retry = true;
        rele1 = NULL;
        rele2 = NULL;
        atomic_inc_uint(&cwdi->cwdi_refcnt);
        mutex_exit(proc_lock);
        rw_enter(&cwdi->cwdi_lock, RW_WRITER);
               
The problem is that p_cwdi IS NOT protected by proc_lock. The exit path
uses reflock to destroy the object.

Since this function is not performance critical, my suggested fix is to
uncondionally take the p_lock and ref cwdi only if the process is alive
and constructed.

Alternatively, cwdi freeing can be split into 2 parts where actual freeing
of the cwdi object itself is postponed until after proc_lock lock/unlock
dance.

Pseudo-code:
struct cwdinfo *
cwddestroy(struct cwdinfo *cwdi)
{
       
        if (atomic_dec_uint_nv(&cwdi->cwdi_refcnt) > 0)
                return NULL;
       
        vrele(cwdi->cwdi_cdir);
        if (cwdi->cwdi_rdir)
                vrele(cwdi->cwdi_rdir);
        if (cwdi->cwdi_edir)
                vrele(cwdi->cwdi_edir);
        return cwdi;
}      

void
cwdfree(struct cwdinfo *cwdi)
{
       
        pool_cache_put(cwdi_cache, cwdi);
}

Then the exit path would:
cwdi = cwddestroy(p->p_cwdi);
p->p_cwdi = NULL;
....
mutex_enter(proc_lock);
....
mutex_exit(proc_lock);
cwdfree(cwdi);

================

mount_checkdirs would then atomic_inc_not_zero (or however you have this
named).

With the above, if cwdi was spotted, it is guaranteeed to not get freed
until after proc_lock is dropped. A successfull non-zero -> non-zero + 1
refcount bump guarantees it wont get freed and the content will remain
valid.

--
Mateusz Guzik <mjguzik gmail.com>


Home | Main Index | Thread Index | Old Index