DF-1061 / fix.diff
diff --git a/sys/platform/pc64/x86_64/pmap_inval.c b/sys/platform/pc64/x86_64/pmap_inval.c --- a/sys/platform/pc64/x86_64/pmap_inval.c +++ b/sys/platform/pc64/x86_64/pmap_inval.c @@ -342,10 +342,26 @@ while (CPUMASK_TESTNZERO(info->done)) { #ifdef LOOPRECOVER if (loopwdog(info)) { - info->failed = 1; - loopdebug("A", info); - /* XXX recover from possible bug */ - CPUMASK_ASSZERO(info->done); + /* + * A target cpu has not acknowledged the PRIOR shootdown + * for >LOOPRECOVER_TIMEOUT1 seconds. Its done bit being + * still set means it has NOT yet executed cpu_invlpg() + * for that command (targets clear done only AFTER invlpg, + * pmap_inval_intr). Force-clearing done here would + * silently lose that target's TLB invalidation: the + * caller would free/remap the page while the lagging cpu + * retains a stale TLB entry -> use-after-free / info + * leak when it resumes. Fail safe instead of corrupting. + * + * Note: info->failed is set but never read anywhere, so + * callers cannot detect the lost invalidation; panicking + * is the only safe recovery. + */ + loopdebug("A-stuck", info); + panic("pmap_inval_smp: cpu %d prior shootdown unacked " + ">%ds, done=%08jx mask=%08jx (stale TLB risk)", + cpu, LOOPRECOVER_TIMEOUT1, info->done.ary[0], + info->mask.ary[0]); } #endif cpu_pause(); @@ -491,10 +507,17 @@ while (CPUMASK_TESTNZERO(info->done)) { #ifdef LOOPRECOVER if (loopwdog(info)) { - info->failed = 1; - loopdebug("B", info); - /* XXX recover from possible bug */ - CPUMASK_ASSZERO(info->done); + /* + * Same rationale as the 'A' path in pmap_inval_smp(): + * force-clearing done loses a target's pending TLB + * invalidation. Fail safe. + */ + loopdebug("B-stuck", info); + panic("pmap_inval_smp_cmpset: cpu %d prior shootdown " + "unacked >%ds, done=%08jx mask=%08jx " + "(stale TLB risk)", + cpu, LOOPRECOVER_TIMEOUT1, info->done.ary[0], + info->mask.ary[0]); } #endif cpu_pause(); |