-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fixed: freed wild pointer cause panic #31
Conversation
please add a test and add build tag linux I think. |
Sure wait me |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need any additional test, this seems to cover it
Line 72 in 9655137
coraza_free_intervention(intervention); |
We don't actually seem to have a CI so separately we would need it eventually.
@@ -234,8 +234,6 @@ func coraza_free_intervention(it *C.coraza_intervention_t) C.int { | |||
return 1 | |||
} | |||
defer C.free(unsafe.Pointer(it)) | |||
C.free(unsafe.Pointer(it.log)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could be just missing it but I don't see log / url fields actually used - should we remove the fields too?
Maybe we should remove it cc @jptosso
…On Fri, 30 Jun 2023, 03:11 potats0, ***@***.***> wrote:
In macos or openresty with arm64 , uninitialized memory will be set zero.
But in apisix 3.3.0 with Ubuntu, will be set 0x1fffffffe. That is why
caused segment fault.
[image: image]
<https://user-images.githubusercontent.com/42128471/249962951-1a0c1dcb-5bf8-4fbb-a53e-9cf9ca33c618.png>
[image: image]
<https://user-images.githubusercontent.com/42128471/249962742-b6af868f-f500-4fc0-87b8-5c4d21c53330.png> [image:
image]
<https://user-images.githubusercontent.com/42128471/249964415-3944ae6c-cc85-464a-81c2-ad1278aa0f47.png>
—
Reply to this email directly, view it on GitHub
<#31 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAXOYASLSRL6B5BPIWU73HLXNYRTNANCNFSM6AAAAAAZYP7PKA>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Any feedback @jptosso ? |
@jptosso These pointers are never set to anything, so the question is whether we can just delete the fields themselves (currently free is being called on them which fails since they're not set to anything) |
@anuraaga I'm ok with it then, in the future we might have to implement them though. |
@potats0 could you please remove |
OK, commited |
In apisix with ubuntu 22.04 arm64, when i called coraza_free_intervention may cause panic. because
it.url
andit.log
are wild pointer. Macos isn't going to happen.