Skip to content

Conversation

Girgias
Copy link
Member

@Girgias Girgias commented Oct 3, 2024

@nielsdos I don't understand why ext/soap/tests/gh12392.phpt fails with use after frees.

But this code is extremely confusing and I'm guessing bug prone as it has recursing behaviour with different functions just calling themselves with the same HashTable.

Not sure if you have any ideas or not.

@nielsdos
Copy link
Member

nielsdos commented Oct 4, 2024

@Girgias this patch is wrong. You may not use the foreach macros if during iteration the hashtable changes. The uafs come from the fact that the bucket pointer is stored in a local variable, and when the hashtable resizes that pointer may become stale.
In fact, I once fixed a bug in soap by changing a foreach macro into these "hash_move" functions.

@nielsdos
Copy link
Member

nielsdos commented Oct 4, 2024

Actually, it's part of this code that I changed from the macro to what it currently is!
So if you would've used "git blame" you could've seen why ;)

@Girgias
Copy link
Member Author

Girgias commented Oct 4, 2024

Ah fair enough, didn't know this fact about HashTables.

But I guess my question is why do the HashTables need to be fixed in the first place?

@nielsdos
Copy link
Member

nielsdos commented Oct 4, 2024

The name of the function is just bad. It looks up for each attribute if there's a reference to another part of the current/another schema, and links them up.

@Girgias Girgias closed this Oct 5, 2024
@Girgias Girgias deleted the soap-hash-foreach branch October 5, 2024 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants