From 0f08abe380a53c4fc3cb23564388c9b9b473ad86 Mon Sep 17 00:00:00 2001 From: Jakub Klimek Date: Fri, 6 Mar 2026 15:29:35 +0100 Subject: [PATCH 1/5] vm/map: fix ENOMEM handling in _vm_mmap Previous handling of ENOMEM during physical page allocation ignored that _map_map can merge new allocation with existing entries, leading to invalid map state, accidental virtual address reallocation and leaking physical pages. JIRA: RTOS-1235 --- vm/map.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/vm/map.c b/vm/map.c index 49e476e2b..3a8dfd323 100644 --- a/vm/map.c +++ b/vm/map.c @@ -624,11 +624,7 @@ void *_vm_mmap(vm_map_t *map, void *vaddr, page_t *p, size_t size, vm_prot_t pro for (w = vaddr; w < vaddr + size; w += SIZE_PAGE) { if (_map_force(map, e, w, prot) != 0) { - amap_putanons(e->amap, e->aoffs, (ptr_t)w - (ptr_t)vaddr); - - (void)pmap_remove(&map->pmap, vaddr, (void *)((ptr_t)w + SIZE_PAGE)); - - _entry_put(map, e); + (void)_vm_munmap(map, vaddr, size); return NULL; } } From c7868171287307003575b9188ce369e8016d5829 Mon Sep 17 00:00:00 2001 From: Jakub Klimek Date: Thu, 12 Mar 2026 10:41:29 +0100 Subject: [PATCH 2/5] vm/map: cleanup on mapCopy failure Previously due to out of memory errors source map and its refcounts could be left in invalid state, leading to memory waste and errors in source process. Also, the dst map was never destroyed. JIRA: RTOS-1235 --- vm/map.c | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/vm/map.c b/vm/map.c index 3a8dfd323..20796ca34 100644 --- a/vm/map.c +++ b/vm/map.c @@ -1116,6 +1116,7 @@ int vm_mapCopy(process_t *proc, vm_map_t *dst, vm_map_t *src) rbnode_t *n; map_entry_t *e, *f; size_t offs; + int err = EOK; (void)proc_lockSet2(&src->lock, &dst->lock); @@ -1149,18 +1150,23 @@ int vm_mapCopy(process_t *proc, vm_map_t *dst, vm_map_t *src) if ((proc == NULL) || (proc->lazy == 0U)) { for (offs = 0; offs < f->size; offs += SIZE_PAGE) { - if (_map_force(dst, f, (void *)((ptr_t)f->vaddr + offs), f->prot) != 0) { - (void)proc_lockClear(&dst->lock); - (void)proc_lockClear(&src->lock); - return -ENOMEM; - } - - if (_map_force(src, e, (void *)((ptr_t)e->vaddr + offs), e->prot) != 0) { - (void)proc_lockClear(&dst->lock); - (void)proc_lockClear(&src->lock); - return -ENOMEM; + err = _map_force(dst, f, (void *)((ptr_t)f->vaddr + offs), f->prot); + if (err != EOK) { + break; } } + if (err != EOK) { + (void)proc_lockClear(&dst->lock); + /* Destroy map before _map_force on source map to ensure refcounts are cleared and avoid unnecessary copies in amap_page */ + vm_mapDestroy(proc, dst); + } + for (offs = 0; offs < e->size; offs += SIZE_PAGE) { + LIB_ASSERT_ALWAYS(_map_force(src, e, (void *)((ptr_t)e->vaddr + offs), e->prot) == EOK, "Broken src map during mapCopy"); + } + if (err != EOK) { + (void)proc_lockClear(&src->lock); + return err; + } } } From b07c5887afac103bb0198c03153cc3dca3095fee Mon Sep 17 00:00:00 2001 From: Jakub Klimek Date: Thu, 12 Mar 2026 10:54:54 +0100 Subject: [PATCH 3/5] vm: error passing from amap/object page fetches The previous notation of returning NULL on error of page allocation was overloaded, as failure to copy page from amap backed by VM_OBJ_PHYSMEM was indistinguishable from VM_OBJ_PHYSMEM outside of defined maps. Introduced error codes to improve maintainability, instead of stacking additional if conditions. JIRA: RTOS-1235 --- vm/amap.c | 41 +++++++++++++++++++++-------------------- vm/amap.h | 2 +- vm/map.c | 9 +++++++-- vm/object.c | 44 ++++++++++++++++++++++++-------------------- vm/object.h | 6 +++++- vm/page.h | 1 + 6 files changed, 59 insertions(+), 44 deletions(-) diff --git a/vm/amap.c b/vm/amap.c index b9bd1e1a8..188c4255a 100644 --- a/vm/amap.c +++ b/vm/amap.c @@ -226,9 +226,9 @@ static int amap_unmap(vm_map_t *map, void *v) } -page_t *amap_page(vm_map_t *map, amap_t *amap, vm_object_t *o, void *vaddr, size_t aoffs, u64 offs, vm_prot_t prot) +int amap_page(vm_map_t *map, amap_t *amap, vm_object_t *o, void *vaddr, size_t aoffs, u64 offs, vm_prot_t prot, page_t **res) { - page_t *p = NULL; + int err = EOK; anon_t *a; void *v, *w; @@ -237,61 +237,61 @@ page_t *amap_page(vm_map_t *map, amap_t *amap, vm_object_t *o, void *vaddr, size a = amap->anons[aoffs / SIZE_PAGE]; if (a != NULL) { (void)proc_lockSet(&a->lock); - p = a->page; + *res = a->page; if (!(a->refs > 1 && (prot & PROT_WRITE) != 0U)) { (void)proc_lockClear(&a->lock); (void)proc_lockClear(&amap->lock); - return p; + return EOK; } a->refs--; } else { - p = vm_objectPage(map, &amap, o, vaddr, offs); - if (p == NULL) { + err = vm_objectPage(map, &amap, o, vaddr, offs, res); + if ((err != EOK) || (*res == NULL)) { /* amap could be invalidated while fetching from the object's store */ if (amap != NULL) { (void)proc_lockClear(&amap->lock); } - return NULL; + return err; } else if (o != NULL && (prot & PROT_WRITE) == 0U) { (void)proc_lockClear(&amap->lock); - return p; + return EOK; } else { /* No action required */ } } - v = amap_map(map, p); + v = amap_map(map, *res); if (v == NULL) { if (a != NULL) { (void)proc_lockClear(&a->lock); } (void)proc_lockClear(&amap->lock); - return NULL; + return -ENOMEM; } if (a != NULL || o != NULL) { /* Copy from object or shared anon */ - p = vm_pageAlloc(SIZE_PAGE, PAGE_OWNER_APP); - if (p == NULL) { + *res = vm_pageAlloc(SIZE_PAGE, PAGE_OWNER_APP); + if (*res == NULL) { (void)amap_unmap(map, v); if (a != NULL) { (void)proc_lockClear(&a->lock); } (void)proc_lockClear(&amap->lock); - return NULL; + return -ENOMEM; } - w = amap_map(map, p); + w = amap_map(map, *res); if (w == NULL) { - vm_pageFree(p); + vm_pageFree(*res); (void)amap_unmap(map, v); if (a != NULL) { (void)proc_lockClear(&a->lock); } (void)proc_lockClear(&amap->lock); - return NULL; + return -ENOMEM; } hal_memcpy(w, v, SIZE_PAGE); (void)amap_unmap(map, w); @@ -306,14 +306,15 @@ page_t *amap_page(vm_map_t *map, amap_t *amap, vm_object_t *o, void *vaddr, size (void)proc_lockClear(&a->lock); } - amap->anons[aoffs / SIZE_PAGE] = anon_new(p); + amap->anons[aoffs / SIZE_PAGE] = anon_new(*res); if (amap->anons[aoffs / SIZE_PAGE] == NULL) { - vm_pageFree(p); - p = NULL; + vm_pageFree(*res); + *res = NULL; + err = -ENOMEM; } (void)proc_lockClear(&amap->lock); - return p; + return err; } diff --git a/vm/amap.h b/vm/amap.h index 3fd2244a3..0f2277fe2 100644 --- a/vm/amap.h +++ b/vm/amap.h @@ -38,7 +38,7 @@ typedef struct _amap_t { } amap_t; -page_t *amap_page(struct _vm_map_t *map, amap_t *amap, struct _vm_object_t *o, void *vaddr, size_t aoffs, u64 offs, vm_prot_t prot); +int amap_page(struct _vm_map_t *map, amap_t *amap, struct _vm_object_t *o, void *vaddr, size_t aoffs, u64 offs, vm_prot_t prot, page_t **res); void amap_clear(amap_t *amap, size_t offset, size_t size); diff --git a/vm/map.c b/vm/map.c index 20796ca34..9b0fd0dc1 100644 --- a/vm/map.c +++ b/vm/map.c @@ -737,6 +737,7 @@ static int _map_force(vm_map_t *map, map_entry_t *e, void *paddr, vm_prot_t prot u64 eoffs; page_t *p = NULL; vm_prot_t flagsCheck = map_checkProt(e->prot, prot); + int err; if (flagsCheck != 0U) { return -EINVAL; @@ -754,10 +755,14 @@ static int _map_force(vm_map_t *map, map_entry_t *e, void *paddr, vm_prot_t prot eoffs = ((e->offs == VM_OFFS_MAX) ? VM_OFFS_MAX : (e->offs + offs)); if (e->amap == NULL) { - p = vm_objectPage(map, NULL, e->object, paddr, eoffs); + err = vm_objectPage(map, NULL, e->object, paddr, eoffs, &p); } else { /* if (e->object != VM_OBJ_PHYSMEM) FIXME disabled until memory objects are created for syspage progs */ - p = amap_page(map, e->amap, e->object, paddr, e->aoffs + offs, eoffs, prot); + err = amap_page(map, e->amap, e->object, paddr, e->aoffs + offs, eoffs, prot, &p); + } + + if (err != EOK) { + return err; } attr = vm_protToAttr(prot) | vm_flagsToAttr(e->flags); diff --git a/vm/object.c b/vm/object.c index 2be97ba92..1bc92ba1f 100644 --- a/vm/object.c +++ b/vm/object.c @@ -207,33 +207,36 @@ static page_t *object_fetch(oid_t oid, u64 offs) } -page_t *vm_objectPage(vm_map_t *map, amap_t **amap, vm_object_t *o, void *vaddr, u64 offs) +int vm_objectPage(vm_map_t *map, amap_t **amap, vm_object_t *o, void *vaddr, u64 offs, page_t **res) { - page_t *p; + int err; if (o == NULL) { - return vm_pageAlloc(SIZE_PAGE, PAGE_OWNER_APP); + *res = vm_pageAlloc(SIZE_PAGE, PAGE_OWNER_APP); + return (*res != NULL) ? EOK : -ENOMEM; } if (o == VM_OBJ_PHYSMEM) { /* parasoft-suppress-next-line MISRAC2012-RULE_14_3 "Check is needed on targets where sizeof(offs) != sizeof(addr_t)" */ if (offs > (addr_t)-1) { - return NULL; + return -ERANGE; } - return _page_get((addr_t)offs); + *res = _page_get((addr_t)offs); + /* res can be NULL, when address outside of defined physical maps is used */ + return EOK; } (void)proc_lockSet(&object_common.lock); if (offs >= o->size) { (void)proc_lockClear(&object_common.lock); - return NULL; + return -EINVAL; } - p = o->pages[offs / SIZE_PAGE]; - if (p != NULL) { + *res = o->pages[offs / SIZE_PAGE]; + if (*res != NULL) { (void)proc_lockClear(&object_common.lock); - return p; + return EOK; } /* Fetch page from backing store */ @@ -246,32 +249,33 @@ page_t *vm_objectPage(vm_map_t *map, amap_t **amap, vm_object_t *o, void *vaddr, (void)proc_lockClear(&map->lock); - p = object_fetch(o->oid, offs); + *res = object_fetch(o->oid, offs); - if (vm_lockVerify(map, amap, o, vaddr, offs) != 0) { - if (p != NULL) { - vm_pageFree(p); + err = vm_lockVerify(map, amap, o, vaddr, offs); + if (err != 0) { + if (*res != NULL) { + vm_pageFree(*res); } - return NULL; + return err; } (void)proc_lockSet(&object_common.lock); if (o->pages[offs / SIZE_PAGE] != NULL) { /* Someone loaded a page in the meantime, use it */ - if (p != NULL) { - vm_pageFree(p); + if (*res != NULL) { + vm_pageFree(*res); } - p = o->pages[offs / SIZE_PAGE]; + *res = o->pages[offs / SIZE_PAGE]; (void)proc_lockClear(&object_common.lock); - return p; + return EOK; } - o->pages[offs / SIZE_PAGE] = p; + o->pages[offs / SIZE_PAGE] = *res; (void)proc_lockClear(&object_common.lock); - return p; + return EOK; } diff --git a/vm/object.h b/vm/object.h index 9faae0a42..585e4d85e 100644 --- a/vm/object.h +++ b/vm/object.h @@ -45,7 +45,11 @@ int vm_objectGet(vm_object_t **o, oid_t oid); int vm_objectPut(vm_object_t *o); -page_t *vm_objectPage(struct _vm_map_t *map, amap_t **amap, vm_object_t *o, void *vaddr, u64 offs); +/* Fills appropriate page_t struct into *res, + * allocates new page when o == NULL + * Note that when o == VM_OBJ_PHYSMEM and offs is outside of defined physical maps, then *res will be NULL even though mapping can still be made + */ +int vm_objectPage(struct _vm_map_t *map, amap_t **amap, vm_object_t *o, void *vaddr, u64 offs, page_t **res); vm_object_t *vm_objectContiguous(size_t size); diff --git a/vm/page.h b/vm/page.h index 9b1841379..28acc6f51 100644 --- a/vm/page.h +++ b/vm/page.h @@ -28,6 +28,7 @@ page_t *vm_pageAlloc(size_t size, vm_flags_t flags); void vm_pageFree(page_t *p); +/* returns NULL when addr is outside of defined physical maps (MMU) */ page_t *_page_get(addr_t addr); From fc0222985a8d05339e02ac0ef818a363c080d0b8 Mon Sep 17 00:00:00 2001 From: Jakub Klimek Date: Wed, 11 Mar 2026 17:38:03 +0100 Subject: [PATCH 4/5] vm/amap: fix refs update when out of memory Hanging refs not only led to wasted resources, but also to invalidating src map in vm_mapCopy due to fail to copy pages that shouldn't be copied during any following map_force. Amap pointer is not updated on failure to avoid hanging anon refs. JIRA: RTOS-1235 --- vm/amap.c | 5 ++--- vm/map.c | 6 ++++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/vm/amap.c b/vm/amap.c index 188c4255a..13fecd17d 100644 --- a/vm/amap.c +++ b/vm/amap.c @@ -118,8 +118,6 @@ amap_t *amap_create(amap_t *amap, size_t *offset, size_t size) (void)proc_lockClear(&amap->lock); return amap; } - - amap->refs--; } /* Allocate anon pointer arrays in chunks @@ -151,6 +149,7 @@ amap_t *amap_create(amap_t *amap, size_t *offset, size_t size) } if (amap != NULL) { + amap->refs--; (void)proc_lockClear(&amap->lock); } @@ -243,7 +242,6 @@ int amap_page(vm_map_t *map, amap_t *amap, vm_object_t *o, void *vaddr, size_t a (void)proc_lockClear(&amap->lock); return EOK; } - a->refs--; } else { err = vm_objectPage(map, &amap, o, vaddr, offs, res); @@ -303,6 +301,7 @@ int amap_page(vm_map_t *map, amap_t *amap, vm_object_t *o, void *vaddr, size_t a (void)amap_unmap(map, v); if (a != NULL) { + a->refs--; (void)proc_lockClear(&a->lock); } diff --git a/vm/map.c b/vm/map.c index 9b0fd0dc1..4148382ca 100644 --- a/vm/map.c +++ b/vm/map.c @@ -737,16 +737,18 @@ static int _map_force(vm_map_t *map, map_entry_t *e, void *paddr, vm_prot_t prot u64 eoffs; page_t *p = NULL; vm_prot_t flagsCheck = map_checkProt(e->prot, prot); + amap_t *amapNew; int err; if (flagsCheck != 0U) { return -EINVAL; } if ((((prot & PROT_WRITE) != 0U) && ((e->flags & MAP_NEEDSCOPY) != 0U)) || ((e->object == NULL) && (e->amap == NULL))) { - e->amap = amap_create(e->amap, &e->aoffs, e->size); - if (e->amap == NULL) { + amapNew = amap_create(e->amap, &e->aoffs, e->size); + if (amapNew == NULL) { return -ENOMEM; } + e->amap = amapNew; e->flags &= ~MAP_NEEDSCOPY; } From 432ea7d244d7d1bd7869e9752409db75832ed979 Mon Sep 17 00:00:00 2001 From: Jakub Klimek Date: Thu, 12 Mar 2026 16:53:13 +0100 Subject: [PATCH 5/5] process: free process_info_t on vfork fail JIRA: RTOS-1235 --- proc/process.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/proc/process.c b/proc/process.c index 0c90f9f76..28af268d0 100644 --- a/proc/process.c +++ b/proc/process.c @@ -1503,6 +1503,12 @@ int proc_vfork(void) (void)vm_objectPut(spawn->object); ret = spawn->state; vm_kfree(spawn); + if ((ret < 0) && (posix_getppid(pid) >= 0)) { + /* if child managed to register itself within posix subsystem before failure, + * wait for its complete death and cleanup its posix metadata. + */ + (void)posix_waitpid(pid, NULL, 0); + } return (ret < 0) ? ret : pid; }