Re: svn commit: r360233 - in head: contrib/jemalloc . . . : This partially breaks a 2-socket 32-bit powerpc (old PowerMac G4) based on head -r360311

[Yet another oddity.]

On 2020-Jun-11, at 21:05, Mark Millard <[email protected]> wrote:
> 
> There is another oddity in the code structure, in
> that if pt was ever NULL the code would misuse the
> NULL before the test for non-NULL is made:
> 
>                pt = moea_pvo_to_pte(pvo, -1);
> . . .
>                old_pte = *pt;
> 
>                /*
>                 * If the PVO is in the page table, update that pte as well.
>                 */
>                if (pt != NULL) {
> 
> (I'm not claiming that this explains the panic.)

There is another NULL handling oddity that the
64-bit code does not have. I'll show 64
relevant bit code first:

        pg = PHYS_TO_VM_PAGE(pvo->pvo_pte.pa & LPTE_RPGN);
        
        . . .
        
        if (. . .&& pg != NULL &&
            (pg->a.flags & PGA_EXECUTABLE) == 0 &&
             . . .) {
                if ((pg->oflags & VPO_UNMANAGED) == 0)
                        vm_page_aflag_set(pg, PGA_EXECUTABLE);
                . . .
        }

        . . .
        if (pg != NULL && . . .) {
                refchg |= atomic_readandclear_32(&pg->md.mdpg_attrs);
                if (refchg & LPTE_CHG)
                        vm_page_dirty(pg);
                if (refchg & LPTE_REF)
                        vm_page_aflag_set(pg, PGA_REFERENCED);
        }

Note: the 2nd outer-if tests for pg != NULL, just like the
first outer-if above does. It avoids potential abuse-of-NULL
activity.

This is not true of the 32-bit code for "m":

                        m = PHYS_TO_VM_PAGE(old_pte.pte_lo & PTE_RPGN);
                        if (. . . && m != NULL &&
                            (m->a.flags & PGA_EXECUTABLE) == 0 &&
                            . . .) {
                                if ((m->oflags & VPO_UNMANAGED) == 0)
                                        vm_page_aflag_set(m, PGA_EXECUTABLE);
                                moea_syncicache(pvo->pvo_pte.pa & PTE_RPGN,
                                    PAGE_SIZE);
                        }
                        . . .
                        if ((pvo->pvo_vaddr & PVO_MANAGED) &&
                            (pvo->pvo_pte.prot & VM_PROT_WRITE)) {
                                refchg = atomic_readandclear_32(&m->md.mdpg_attrs);
                                if (refchg & PTE_CHG)
                                        vm_page_dirty(m);
                                if (refchg & PTE_REF)
                                        vm_page_aflag_set(m, PGA_REFERENCED);
                        }

The &m->md.mdpg_attrs use apparently could be based on m
being NULL because there is no test for m != NULL in the
2nd outer-if. Similarly for the other uses of m in that
code block.

My guess that that the 2nd outer-if should test something
like:

. . .
                        if (m != NULL &&
                            (pvo->pvo_vaddr & PVO_MANAGED) &&
                            (pvo->pvo_pte.prot & VM_PROT_WRITE)) {
. . .

(Presumes that the pre-existing m != NULL tests are
necessary, something that I do not know.)


===
Mark Millard
marklmi at yahoo.com
( dsl-only.net went
away in early 2018-Mar)

_______________________________________________
[email protected] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "[email protected]"