pmap-v6.c has a bug?

[Kohji Okuno]

Hi All,

We think that pmap_alloc_l2_bucket() in pmap-v6.c has a bug for a
race-condition.

Would you refer to the following "(***)" lines?

When a context(called A) decides to allocate pte after A checks l2
and l2b->kva, A releases locks while A is allocating pte. In this
timing, another context(called B) may free the same l2 from
pmap_free_l2_bucket(). If this situation happens, l2b which is
allocated by A will be lost since this l2b isn't able to trace from
pmap.

In this result, pmap_get_l2_bucket(pvchunk->pc_pmap, pventry->pv_va)
will return NULL, then it will cause a kernel panic by NULL access.
We saw this kind of panic in pmap_clearbits() and pmap_remove_all().

We add count-up l2_occupancy before unloking and count-down it after
locking. We think that this change can prevent wrong release of l2.

What do you think about this?

Regards,
 Kohji Okuno

707 static struct l2_bucket *
708 pmap_alloc_l2_bucket(pmap_t pmap, vm_offset_t va)
709 {
710         struct l2_dtable *l2;
711         struct l2_bucket *l2b;
712         u_short l1idx;
713
714         l1idx = L1_IDX(va);
715
716         PMAP_ASSERT_LOCKED(pmap);
717         rw_assert(&pvh_global_lock, RA_WLOCKED);
718         if ((l2 = pmap->pm_l2[L2_IDX(l1idx)]) == NULL) {
		>> SNIP <<
747         }
748
749         l2b = &l2->l2_bucket[L2_BUCKET(l1idx)];
750
751         /*
752          * Fetch pointer to the L2 page table associated with the address.
753          */
754         if (l2b->l2b_kva == NULL) {
755                 pt_entry_t *ptep;
756
757                 /*
758                  * No L2 page table has been allocated. Chances are, this
759                  * is because we just allocated the l2_dtable, above.
760                  */
(***)               l2->l2_occupancy++;	/* prevent release of l2 */
761                 PMAP_UNLOCK(pmap);
762                 rw_wunlock(&pvh_global_lock);
763                 ptep = uma_zalloc(l2zone, M_NOWAIT);
764                 rw_wlock(&pvh_global_lock);
765                 PMAP_LOCK(pmap);
(***)               l2->l2_occupancy--;
766                 if (l2b->l2b_kva != 0) {
767                         /* We lost the race. */
768                         uma_zfree(l2zone, ptep);
769                         return (l2b);
770                 }
771                 l2b->l2b_phys = vtophys(ptep);
772                 if (ptep == NULL) {
773                         /*
774                          * Oops, no more L2 page tables available at this
775                          * time. We may need to deallocate the l2_dtable
776                          * if we allocated a new one above.
777                          */
778                         if (l2->l2_occupancy == 0) {
779                                 pmap->pm_l2[L2_IDX(l1idx)] = NULL;
780                                 uma_zfree(l2table_zone, l2);
781                         }
782                         return (NULL);
783                 }
784
785                 l2->l2_occupancy++;
786                 l2b->l2b_kva = ptep;
787                 l2b->l2b_l1idx = l1idx;
788         }
789
790         return (l2b);
791 }