vm_map.c: plug several more places which might modify entry->offset
ClosedPublic
Actions

Authored by kib on Aug 15 2023, 8:52 PM.

Details

Reviewers

alc
markj
dougm

Commits

rGc718009884b3: vm_map.c: plug several more places which might modify entry->offset

Summary

for the GUARD entries protecting stacks gaps.

syzkaller: https://syzkaller.appspot.com/bug?extid=c325d6a75e4fd0a68714

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

kib created this revision.Aug 15 2023, 8:52 PM

Herald added a subscriber: imp. · View Herald TranscriptAug 15 2023, 8:52 PM

kib requested review of this revision.Aug 15 2023, 8:52 PM

dougm added inline comments.Aug 16 2023, 3:59 AM

sys/vm/vm_map.c
1418	Have you tested this change with INVARIANTS set? It seems that if you don't change root->offset, then you'll have two elements overlap, and that will violate invariants.

kib added inline comments.Aug 16 2023, 8:58 AM

sys/vm/vm_map.c
1418	Peter is doing the full test right now. offset is overloaded for stack gap entries, this is the main point of the change. Are you saying that overloading entry->offset cannot work in principle?

dougm added inline comments.Aug 16 2023, 5:18 PM

sys/vm/vm_map.c
1418	No, I'm not saying that. I withdraw my question. Where is the overloading of entry->offset documented?

kib added inline comments.Aug 16 2023, 5:29 PM

sys/vm/vm_map.c
1418	The comment above the struct vm_map_entry definition in vm_map.h, and duplicated in vm_map_stack_locked() in the KERN_SUCCESS branch of the last if().

Does the second loop in vm_map_sync() also need a check for stack guards?

In D41475#944878, @markj wrote:

Does the second loop in vm_map_sync() also need a check for stack guards?

vm_object_reference()/dereference()/sync() all accept and do nothing if passed obj is NULL.

dougm added inline comments.Aug 16 2023, 7:28 PM

sys/vm/vm_map.c
1466	Do you need a check here?

kib added inline comments.Aug 16 2023, 7:32 PM

sys/vm/vm_map.c
1466	I do not believe so because stack guards are not merge-able. I can add assert.

markj accepted this revision.Aug 16 2023, 8:04 PM

This revision is now accepted and ready to land.Aug 16 2023, 8:04 PM

dougm accepted this revision.Aug 16 2023, 10:18 PM

Closed by commit rGc718009884b3: vm_map.c: plug several more places which might modify entry->offset (authored by kib). · Explain WhyAug 18 2023, 12:44 PM

This revision was automatically updated to reflect the committed changes.

kib added a commit: rGc718009884b3: vm_map.c: plug several more places which might modify entry->offset.

Revision Contents
Changeset List

Path

Size

sys/

vm/

vm_map.c

13 lines

Diff 126202

View Options

vm_map.c: plug several more places which might modify entry->offsetClosedPublicActions