Skip to content

Fix use-after-free in filter() / map() plus some other stuff#407

Open
MattCatz wants to merge 7 commits into
jow-:masterfrom
MattCatz:master
Open

Fix use-after-free in filter() / map() plus some other stuff#407
MattCatz wants to merge 7 commits into
jow-:masterfrom
MattCatz:master

Conversation

@MattCatz

@MattCatz MattCatz commented Apr 28, 2026

Copy link
Copy Markdown

This should address: #406

While working on the original issue I came across a few other gc related things and have also included them.

I tried to break everything down into its own commit in case you want to drop one of the fixes for something better. I added a test case for each of the issues. You should be able to recreate the issues locally by running the test cases with ASAN or valgrind (I used ASAN locally).

Open to feedback.

@MattCatz

Copy link
Copy Markdown
Author

I believe the CI check is a false alarm. It seems to be fixed in newer versions of gcc (CI uses gcc10?). I'll make a push to keep CI happy.

@nbd168

nbd168 commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

I think the fixup should be folded into the commit. Also the json fix would probably be a bit cleaner with a goto out instead of the duplicated ucv_put calls. Other than that, this PR looks good to me.

MattCatz added 7 commits June 10, 2026 11:26
`ucv_array_new` returns an array with a ref count of 1. `ucv_get(cbs)`
adds an extra reference and the array is never free-ed.

Signed-off-by: Matthew Cather <mattbob4@gmail.com>
53 - filter/map/callfunc: result/scope freed during a callback gc("collect")
54 - json_from_object: cached read method dangles after obj.read reassignment
55 - zlib def_object/inf_object
56 - socket.poll: result array freed during a user fileno() callback
57 - uvcount=0 resources leaked when reached via a retain=true sweep

Signed-off-by: Matthew Cather <mattbob4@gmail.com>
uc_json_from_object cached the read method via ucv_property_get,
which returns a borrow. If the user's read callback reassigned
obj.read, the original closure's last owning reference vanished
when the call frame popped, leaving the cached pointer dangling
for the next iteration's ucv_get.

Signed-off-by: Matthew Cather <mattbob4@gmail.com>
uc_filter and uc_map hold their result array in a C local; uc_callfunc
holds the saved previous globals scope. A gc("collect") triggered from
inside the user callback freed those values and left the builtin to
dereference a dangling pointer once control returned.

Push each onto the VM stack with ucv_get so they're rooted via vm->stack
across the call.

Signed-off-by: Matthew Cather <mattbob4@gmail.com>
A gc("collect") from inside a polled handle's fileno() method freed
the C-local result array out from under uc_socket_poll. Push it onto
the VM stack so it stays rooted across the call.

uc_fn_arg(i) peeks from the top, so it now sees the anchor instead of
the args; switch the loop to direct stack-index access via argoff.

Signed-off-by: Matthew Cather <mattbob4@gmail.com>
uc_zlib_def_object and uc_zlib_inf_object cached the read method via
ucv_property_get, which returns a borrow. If the user's read callback
reassigned obj.read, the original closure's last owning reference
vanished when the callframe popped, leaving the cached pointer dangling
for the next iteration's ucv_get.

Take an owning reference and release it at every exit path.

Signed-off-by: Matthew Cather <mattbob4@gmail.com>
ucv_free's retain=true path defers free() and expects the GC's second
pass to mop up UC_NULL items from vm->values. But resources with
uvcount=0 aren't on that list — ucv_resource_new_ex skips them because
leaf resources can't form cycles. So a retain=true sweep cascading into
one set type=UC_NULL and walked away; the memory leaked forever.

Only defer when ref is actually linked into a list.

Signed-off-by: Matthew Cather <mattbob4@gmail.com>
@MattCatz

Copy link
Copy Markdown
Author

Let me know if you see anything else @nbd168

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants