Skip to content

Commit c119f03

Browse files
committed
fix: (COW) 2 severe bugs related to uvmcopy & copyout
1. uvmcopy incorrectly marking readonly pages as COW 2. multi-page copyout wreaking shared pages after the first page due to lack of COW checking for subsequent pages
1 parent 3e4542e commit c119f03

File tree

1 file changed

+13
-6
lines changed

1 file changed

+13
-6
lines changed

kernel/vm.c

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -321,16 +321,24 @@ uvmcopy(pagetable_t old, pagetable_t new, uint64 sz)
321321
if((*pte & PTE_V) == 0)
322322
panic("uvmcopy: page not present");
323323
pa = PTE2PA(*pte);
324-
// clear out PTE_W for parent, set PTE_COW
325-
*pte = (*pte & ~PTE_W) | PTE_COW;
324+
if(*pte & PTE_W) {
325+
// clear out PTE_W for parent, set PTE_COW
326+
*pte = (*pte & ~PTE_W) | PTE_COW;
327+
}
326328
flags = PTE_FLAGS(*pte);
327329
// map physical page of parent directly to child (copy-on-write)
328330
// since the write flag has already been cleared for the parent
329331
// the child mapping won't have the write flag as well.
332+
//
333+
// for page that is already read-only for parent, it will be read-
334+
// only for child as well.
335+
// for read-only page that is also a cow page, the PTE_COW flag will
336+
// be copied over to child page, making it a cow page automatically.
330337
if(mappages(new, i, PGSIZE, (uint64)pa, flags) != 0){
331338
goto err;
332339
}
333-
// increase reference count of the page by one (for the child)
340+
// for any cases above, we created a new reference to the physical
341+
// page, so increase reference count by one.
334342
krefpage((void*)pa);
335343
}
336344
return 0;
@@ -361,10 +369,9 @@ copyout(pagetable_t pagetable, uint64 dstva, char *src, uint64 len)
361369
{
362370
uint64 n, va0, pa0;
363371

364-
if(uvmcheckcowpage(dstva))
365-
uvmcowcopy(dstva);
366-
367372
while(len > 0){
373+
if(uvmcheckcowpage(dstva))
374+
uvmcowcopy(dstva);
368375
va0 = PGROUNDDOWN(dstva);
369376
pa0 = walkaddr(pagetable, va0);
370377
if(pa0 == 0)

0 commit comments

Comments
 (0)