Fix use after free ebpf_object_t code #2311
Conversation
Alan-Jowett
requested review from
dthaler,
poornagmsft,
saxena-anurag,
shankarseal,
dv-msft,
delaramamiri and
gtrevi
as code owners
|
This PR triggers a deadlock due to latent bug in _ebpf_object_release_reference_under_lock. |
Fixed in latest version. |
Signed-off-by: Alan Jowett <alanjo@microsoft.com>
Signed-off-by: Alan Jowett <alanjo@microsoft.com>
Signed-off-by: Alan Jowett <alanjo@microsoft.com>
Signed-off-by: Alan Jowett <alanjo@microsoft.com>
Co-authored-by: Dave Thaler <dthaler@microsoft.com>
Signed-off-by: Alan Jowett <alanjo@microsoft.com>
| uint32_t marker; | ||
| volatile int32_t reference_count; | ||
| uint32_t marker; // Contains the 32bit value 'eobj' when the object is valid and is inverted when the object is | ||
| // freed. |
There was a problem hiding this comment.
The marker can have other values also:
| @@ -263,8 +263,10 @@ ebpf_native_release_reference(_In_opt_ _Post_invalid_ ebpf_native_module_t* modu | |||
|
|
|||
| ebpf_assert(module->base.marker == _ebpf_native_marker); | |||
|
|
|||
| new_ref_count = ebpf_interlocked_decrement_int32(&module->base.reference_count); | |||
| ebpf_assert(new_ref_count != -1); | |||
| new_ref_count = ebpf_interlocked_decrement_int64(&module->base.reference_count); | |||
There was a problem hiding this comment.
A similar problem then exists here also. I think we will need to acquire hash table lock here before calling ebpf_interlocked_decrement_int64()
There was a problem hiding this comment.
#2326 is tracking the ref-count issue in the native module code. I need to better understand the ref-counting model used by the native module code before I can fix it.
There was a problem hiding this comment.
For my info -- how did we hit these issues? any manual testing or new tests in CICD?
github-merge-queue
bot
removed this pull request from the merge queue due to no response for status checks
Resolves: #2276
Description
Release reference in the ebpf_object code decrements the ref-count outside of the lock, which can result in entries being in the object tracking list with ref-count of 0. This can result in object's ref-counts going from 1 -> 0 -> 1 and the object being deleted.
Testing
CI/CD + KM stress.
Documentation
No.