pmap attempts at copying to executable pages both on mips and powerpc/booke

[Rin Okuyama]

I reported a reproducible failure of KASSERT on powerpc/booke in which
destination of pmap_copy_page(9) is executable:

https://mail-index.netbsd.org/port-powerpc/2016/09/11/msg003498.html

By adding the similar KASSERTs to mips kernel, I observed the same
failure on ERLITE (evbmips64-eb):

# uname -mpr
7.99.38 evbmips mips64eb
# cd /usr/pkgsrc/lang/perl5; make
(snip)
=> Checking for portability problems in extracted files
(snip)
Use which C compiler? [gcc]
execve_loadvm: check exec failed 8
execve_loadvm: check exec failed 8
execve_loadvm: check exec failed 8
execve_loadvm: check exec failed 8
Checking for GNU cc in disguise and/or its version number...
panic: kernel diagnostic assertion "!VM_PAGEMD_EXECPAGE_P(VM_PAGE_TO_MD(dst_pg))" failed: file "/var/build/src/sys/arch/mips/mips/pmap_machdep.c", line 628
kernel: breakpoint trap
Stopped in pid 2328.1 (sed) at netbsd:cpu_Debugger+0x4: jr ra
bdslot: nop

This is also reproducible, and occurs at the exact same place as in
the case of powerpc/booke. Something must be wrong with the MI parts
of pmap.

I attached the patch for mips kernel, as well as ps and bt in a DDB
session below.

Thanks,
Rin
====
--- src/sys/arch/mips/mips/pmap_machdep.c.orig 2016-09-12 16:08:10.605295167 +0900
+++ src/sys/arch/mips/mips/pmap_machdep.c 2016-09-12 16:16:11.232876559 +0900
@@ -595,6 +595,7 @@

struct vm_page * const dst_pg = PHYS_TO_VM_PAGE(dst_pa);

+ KASSERT(!VM_PAGEMD_EXECPAGE_P(VM_PAGE_TO_MD(dst_pg)));
const register_t dst_va = pmap_md_map_ephemeral_page(dst_pg, false,
VM_PROT_READ|VM_PROT_WRITE, &dst_pte);

@@ -624,6 +625,7 @@
VM_PROT_READ, &src_pte);

KASSERT(VM_PAGE_TO_MD(dst_pg)->mdpg_first.pv_pmap == NULL);
+ KASSERT(!VM_PAGEMD_EXECPAGE_P(VM_PAGE_TO_MD(dst_pg)));
const register_t dst_va = pmap_md_map_ephemeral_page(dst_pg, false,
VM_PROT_READ|VM_PROT_WRITE, &dst_pte);

====
db> ps
PID LID S CPU FLAGS STRUCT LWP * NAME WAIT
2328 > 1 7 0 0 ffffffff8f9f5620 sed
2092 1 3 0 80 ffffffff8fe52920 sh wait
1117 1 3 0 80 ffffffff8feb4080 sh pipe_rd
1108 1 3 0 80 ffffffff8feb44c0 sh wait
764 1 3 0 80 ffffffff8fe531a0 make wait
754 1 3 0 80 ffffffff8fe535e0 sh wait
575 1 3 0 80 ffffffff8fb40d80 make wait
515 1 3 0 80 ffffffff8fb41a40 sh wait
437 1 3 0 80 ffffffff8ff28060 login wait
542 1 3 0 80 ffffffff8f9f51e0 cron nanoslp
592 1 3 0 80 ffffffff8fe524e0 qmgr kqueue
465 1 3 0 80 ffffffff8fb41600 pickup kqueue
569 1 3 0 80 ffffffff8f9f5a60 master kqueue
342 1 3 0 80 ffffffff8fb40940 sshd select
331 1 3 0 80 ffffffff8fe52d60 ntpd netio
314 1 3 0 80 ffffffff8fb411c0 ntpd pause
147 1 2 0 0 ffffffff8fe520a0 syslogd
1 1 3 0 80 ffffffff8ff299e0 init wait
0 42 3 0 200 ffffffff8feb5a00 swapiod swapiod
0 41 3 0 200 ffffffff8feb4d40 aiodoned aiodoned
0 40 3 0 200 ffffffff8feb5180 ioflush syncer
0 39 3 0 200 ffffffff8feb55c0 pgdaemon pgdaemon
0 36 3 0 200 ffffffff8ff284a0 nfsio nfsiod
0 35 3 0 200 ffffffff8ff288e0 nfsio nfsiod
0 34 3 0 200 ffffffff8ff308c0 nfsio nfsiod
0 33 3 0 200 ffffffff8ff30480 nfsio nfsiod
0 31 3 0 200 ffffffff8ff30d00 scsibus0 sccomp
0 30 3 0 200 ffffffff8ff30040 usb0 usbevt
0 29 3 0 200 ffffffff8ff295a0 unpgc unpgc
0 28 3 0 200 ffffffff8ff29160 rt_timer rt_timer
0 27 3 0 200 ffffffff8ff28d20 vmem_rehash vmem_rehash
0 18 3 0 200 ffffffff8ff31140 usbtask-dr usbtsk
0 17 3 0 200 ffffffff8ff31580 usbtask-hc usbtsk
0 16 3 0 200 ffffffff8ff319c0 dwc2 dwc2
0 15 3 0 200 ffffffff8ff64020 pmfsuspend pmfsuspend
0 14 3 0 200 ffffffff8ff64460 pmfevent pmfevent
0 13 3 0 200 ffffffff8ff648a0 sopendfree sopendfr
0 12 3 0 200 ffffffff8ff64ce0 nfssilly nfssilly
0 11 3 0 200 ffffffff8ff65120 cachegc cachegc
0 10 3 0 200 ffffffff8ff65560 vrele vrele
0 9 3 0 200 ffffffff8ff659a0 vdrain vdrain
0 8 3 0 200 ffffffff8ff76000 modunload mod_unld
0 7 3 0 200 ffffffff8ff76440 xcall/0 xcall
0 6 1 0 200 ffffffff8ff76880 softser/0
0 5 1 0 200 ffffffff8ff76cc0 softclk/0
0 4 1 0 200 ffffffff8ff77100 softbio/0
0 3 1 0 200 ffffffff8ff77540 softnet/0
0 2 1 0 201 ffffffff8ff77980 idle/0
0 1 3 0 200 ffffffff80506a40 swapper uvm
db> bt
0x9800000410991980: cpu_Debugger+4 (63061,ff00,0,ffffffff804fcaf0) ra ffffffff8039ef1c sz 0
0x9800000410991980: vpanic+15c (63061,ff00,0,ffffffff804fcaf0) ra ffffffff80447e38 sz 64
0x98000004109919c0: kern_assert+48 (63061,ffffffff80463740,ffffffff8046aa88,ffffffff8046a948) ra ffffffff8022d730 sz 96
0x9800000410991a20: pmap_copy_page+100 (63061,ffffffff80463740,ffffffff8046aa88,ffffffff8046a948) ra ffffffff80315190 sz 48
0x9800000410991a50: uvmfault_promote+150 (63061,ffffffff80463740,ffffffff80d75b00,ffffffff8046a948) ra ffffffff80316608 sz 96
0x9800000410991ab0: uvm_fault_internal+bf0 (63061,ffffffff80463740,2,0) ra ffffffff8023315c sz 512
0x9800000410991cb0: trap+89c (63061,1,2,787d1f80) ra ffffffff8022842c sz 496
0x9800000410991ea0: mips64r2_user_gen_exception+cc (63061,1,2,787d1f80) ra 0 sz 0

User-level: pid 2328.1

[Nick Hudson]

On 09/16/16 14:19, Rin Okuyama wrote:
> I reported a reproducible failure of KASSERT on powerpc/booke in which
> destination of pmap_copy_page(9) is executable:
>
> https://mail-index.netbsd.org/port-powerpc/2016/09/11/msg003498.html
>
> By adding the similar KASSERTs to mips kernel, I observed the same
> failure on ERLITE (evbmips64-eb):
>
> # uname -mpr
> 7.99.38 evbmips mips64eb
> # cd /usr/pkgsrc/lang/perl5; make
> (snip)
> => Checking for portability problems in extracted files
> (snip)
> Use which C compiler? [gcc]
> execve_loadvm: check exec failed 8
> execve_loadvm: check exec failed 8
> execve_loadvm: check exec failed 8
> execve_loadvm: check exec failed 8
> Checking for GNU cc in disguise and/or its version number...
> panic: kernel diagnostic assertion
> "!VM_PAGEMD_EXECPAGE_P(VM_PAGE_TO_MD(dst_pg))" failed: file
> "/var/build/src/sys/arch/mips/mips/pmap_machdep.c", line 628
> kernel: breakpoint trap
> Stopped in pid 2328.1 (sed) at netbsd:cpu_Debugger+0x4: jr ra
> bdslot: nop
>

Matt fixed it with src/sys/uvm/pmap/pmap.c:1.22

Nick

[Rin Okuyama]

Sorry for my long silence.

Thanks to Matt's fix, I succeeded to build perl5 without kernel panic both
on mips and powerpc/booke.

However, unfortunately, something is still wrong. top(1) reports resources
of some processes are negative:

% top
...
PID USERNAME PRI NICE SIZE RES STATE TIME WCPU CPU COMMAND
...
573 root 85 0 4304K -3832K nanoslp 0:01 0.00% 0.00% cron
...

The simplest example to reproduce this error may be the following script:

#!/bin/sh
for i in `seq 1 100`; do
echo $i | echo > /dev/null
done
echo 'hit enter to exit'
read i

top reports resource of sh executing this script is negative, both on
mips and powerpc/booke. On the other hand, the following scripts do *not*
cause the problem:

#!/bin/sh
for i in `seq 1 100`; do
echo $i > /dev/null
done
echo 'hit enter to exit'
read i

#!/bin/sh
yes | echo > /dev/null

I therefore guess that this problem occurs when pipes are created many
times, regardless of amount of data flow.

At the moment, I don't know whether this problem is related to pmap or
not. But, at least, it occurs on mips and powerpc/booke, whereas it does
not on amd64, arm/allwinner, and powerpc/ibm4xx.

Thanks,
Rin

[Nick Hudson]

On 09/20/16 12:34, Rin Okuyama wrote:

[snip...]

> However, unfortunately, something is still wrong. top(1) reports
> resources
> of some processes are negative:
>
> % top
> ...
> PID USERNAME PRI NICE SIZE RES STATE TIME WCPU CPU COMMAND
> ...
> 573 root 85 0 4304K -3832K nanoslp 0:01 0.00% 0.00% cron
> ...
>
>

The problem is here...

https://nxr.netbsd.org/xref/src/sys/uvm/pmap/pmap.c#1275

1275 if (pte_valid_p(opte) && pte_to_paddr(opte) != pa) {
1276 pmap_remove(pmap, va, va + NBPG);
1277 PMAP_COUNT(user_mappings_changed);
1278 }
1279
1280 KASSERT(pte_valid_p(npte));
1281 const bool resident = pte_valid_p(opte);
1282 if (resident) {
1283 update_flags |= PMAP_TLB_NEED_IPI;
1284 } else {
1285 pmap->pm_stats.resident_count++;
1286 }

if the mapping changes PA then the resident_count gets reduced by
pmap_{,pte_}remove, but not increased again.

I'm working on a fix.

Nick

[Rin Okuyama]

On 2016/09/28 20:40, Nick Hudson wrote:
> The problem is here...
>
> https://nxr.netbsd.org/xref/src/sys/uvm/pmap/pmap.c#1275
>
> 1275 if (pte_valid_p(opte) && pte_to_paddr(opte) != pa) {
> 1276 pmap_remove(pmap, va, va + NBPG);
> 1277 PMAP_COUNT(user_mappings_changed);
> 1278 }
> 1279
> 1280 KASSERT(pte_valid_p(npte));
> 1281 const bool resident = pte_valid_p(opte);
> 1282 if (resident) {
> 1283 update_flags |= PMAP_TLB_NEED_IPI;
> 1284 } else {
> 1285 pmap->pm_stats.resident_count++;
> 1286 }
>
> if the mapping changes PA then the resident_count gets reduced by pmap_{,pte_}remove, but not increased again.
>
> I'm working on a fix.

Thank you very much for your analysis. I'm looking forward to hearing
good news from you!

Rin

[Nick Hudson]

On 09/28/16 16:13, Rin Okuyama wrote:
> On 2016/09/28 20:40, Nick Hudson wrote:
>>

[snip]

>>
>> if the mapping changes PA then the resident_count gets reduced by
>> pmap_{,pte_}remove, but not increased again.
>>
>> I'm working on a fix.
>
> Thank you very much for your analysis. I'm looking forward to hearing
> good news from you!
>
> Rin
>

Fixed in sys/uvm/pmap/pmap.c:1.23

Nick

[Rin Okuyama]

On 2016/09/30 21:11, Nick Hudson wrote:
> Fixed in sys/uvm/pmap/pmap.c:1.23

Fix confirmed both on mips64-eb and powerpc/booke.
Thank you so much for your great hack!

Rin