vm/map: implement mapBelongs for NOMMU by etiaro · Pull Request #797 · phoenix-rtos/phoenix-rtos-kernel

etiaro · 2026-06-24T14:21:20Z

Description

Improve separation between processes with different physical maps.
Verifying if provided memory is allocated at all can simplify debugging user applications.

Motivation and Context

Improves future partitioning isolation.

Fixes phoenix-rtos/phoenix-rtos-project#1593

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Chore (refactoring, style fixes, git/CI config, submodule management, no code logic changes)

How Has This Been Tested?

Already covered by automatic testing.
New test added: (add PR link here).
Tested by hand on: (list targets here).

Checklist:

My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
All new and existing linter checks and tests passed.
My changes generate no new compilation warnings for any of the targets.

Special treatment

This PR needs additional PRs to work (list the PRs, preferably in merge-order).
I will merge this PR by myself when appropriate.

gemini-code-assist

Code Review

This pull request updates the _vm_mapBelongs function in vm/map.c to support memory map validation on NOMMU architectures. It introduces checks to verify that the memory is accessible from userspace and resolves the map using syspage_mapAddrResolve and vm_getSharedMap. The review feedback correctly identifies a potential null pointer dereference when calling vm_getSharedMap and suggests adding a safety check before accessing its tree member.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

etiaro · 2026-06-24T14:28:38Z

/gemini review

gemini-code-assist

Code Review

This pull request enables memory map validation on NOMMU systems within _vm_mapBelongs by checking userspace accessibility and resolving the memory range against shared maps. The review feedback highlights critical issues in this implementation, including a potential data race due to missing locks when querying a shared map, a missing boundary check for the resolved map's end, and a missing integer overflow check on the input memory range (ptr + size).

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

github-actions · 2026-06-24T14:59:19Z

Unit Test Results

10 860 tests ±0 10 190 ✅ ±0 53m 26s ⏱️ +24s
670 suites ±0 670 💤 ±0
1 files ±0 0 ❌ ±0

Results for commit b8caaf9. ± Comparison against base commit 1ddeed8.

♻️ This comment has been updated with latest results.

etiaro · 2026-06-24T15:34:18Z

/gemini review

gemini-code-assist

Code Review

This pull request refactors _vm_mapBelongs to map_belongs, moving the locking mechanism inside the function and adding support for NOMMU systems by resolving shared maps. It also introduces an overflow check for the memory range. The review feedback highlights a critical security vulnerability where lib_rbFind only checks for range overlap rather than full containment, potentially allowing access to unmapped or kernel memory. Additionally, checking the found entry outside the locked section poses a use-after-free risk. It is recommended to perform the containment check within the locked sections as suggested.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Improve separation between processes with different physical maps. Verifying if provided memory is allocated at all can simplify debugging user applications. TASK: RTOS-1365

etiaro · 2026-06-25T09:28:00Z

Containment and overflow checks suggested by gemini will be introduced in separate PR, as they break some CI tests.

etiaro · 2026-06-25T11:02:45Z

It's worth to notice this check introduces a silent assumption that checked region is contained within a single map.
I guess this assumption is reasonable, but correct me if I'm missing something.

julianuziemblo

Could you also add simple test to test the case where map_belongs should fail and the syscall produce -EFAULT (on NOMMU and MMU)?

etiaro · 2026-06-26T11:22:02Z

Could you also add simple test to test the case where map_belongs should fail and the syscall produce -EFAULT (on NOMMU and MMU)?

This is problematic, as currently kernel hangs on assertion when mapBelongs check fails in debug mode.
Our test suite doesn't seem to support checking for a system hang as the expected test behavior.

julianuziemblo · 2026-06-26T12:05:43Z

-		f = lib_treeof(map_entry_t, linkage, lib_rbFind(&proc->imapp->tree, &e.linkage));
-	}
+	(void)proc_lockSet(&map->lock);
+	f = lib_treeof(map_entry_t, linkage, lib_rbFind(&map->tree, &e.linkage));


so here, map is proc->mapp on MMU and the shared map resolved from syspage on NOMMU, yes?

gemini-code-assist Bot reviewed Jun 24, 2026

View reviewed changes

Comment thread vm/map.c Outdated

etiaro force-pushed the etiaro/nommu-mapbelongs branch from 4bfd909 to 8fc7315 Compare June 24, 2026 14:27

gemini-code-assist Bot reviewed Jun 24, 2026

View reviewed changes

Comment thread vm/map.c Outdated

Comment thread vm/map.c

github-advanced-security AI found potential problems Jun 24, 2026

View reviewed changes

Comment thread vm/map.c Fixed

etiaro force-pushed the etiaro/nommu-mapbelongs branch from 8fc7315 to ae7ca32 Compare June 24, 2026 15:33

gemini-code-assist Bot reviewed Jun 24, 2026

View reviewed changes

Comment thread vm/map.c Outdated

etiaro force-pushed the etiaro/nommu-mapbelongs branch 3 times, most recently from 30acb23 to 88a2776 Compare June 24, 2026 16:35

vm/map: implement mapBelongs for NOMMU

b8caaf9

Improve separation between processes with different physical maps. Verifying if provided memory is allocated at all can simplify debugging user applications. TASK: RTOS-1365

etiaro force-pushed the etiaro/nommu-mapbelongs branch from 88a2776 to b8caaf9 Compare June 25, 2026 09:22

etiaro marked this pull request as ready for review June 25, 2026 09:52

etiaro requested review from a team and agkaminski June 25, 2026 09:52

julianuziemblo reviewed Jun 25, 2026

View reviewed changes

damianloew mentioned this pull request Jun 25, 2026

NOMMU: vm_mapBelongs() is a no-op - syscall pointer validation silently passes invalid user pointers phoenix-rtos/phoenix-rtos-project#1677

Open

etiaro requested a review from a team June 25, 2026 11:41

julianuziemblo reviewed Jun 26, 2026

View reviewed changes

Uh oh!

Conversation

etiaro commented Jun 24, 2026

Description

Motivation and Context

Types of changes

How Has This Been Tested?

Checklist:

Special treatment

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

etiaro commented Jun 24, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Unit Test Results

Uh oh!

etiaro commented Jun 24, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

etiaro commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

etiaro commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

julianuziemblo left a comment

Choose a reason for hiding this comment

Uh oh!

etiaro commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

julianuziemblo Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

etiaro Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Jun 24, 2026 •

edited

Loading

etiaro commented Jun 25, 2026 •

edited

Loading

etiaro commented Jun 25, 2026 •

edited

Loading

etiaro commented Jun 26, 2026 •

edited

Loading