[Home]

Summary:ASTERISK-30490: stasis: Deleting bridge multiple times at same time may cause crash
Reporter:Yash Khandelwal (Yashyk456)Labels:patch stasis
Date Opened:2023-04-08 07:31:00Date Closed:
Priority:MajorRegression?
Status:Open/NewComponents:Core/Stasis
Versions:18.9.0 Frequency of
Occurrence		Occasional
Related
Issues:
Environment:Ubuntu-18.04Attachments:( 0) stasis.patch
Description:Issue while deleting the bridge :

Getting the core dump

{noformat}
#0  __strncmp_avx2 () at ../sysdeps/x86_64/multiarch/strcmp-avx2.S:102
#1  0x00005636f48bdecb in stasis_topic_pool_delete_topic (pool=0x5636f70bd0d8, topic_name=0x25 <error: Cannot access memory at address 0x25>) at stasis.c:1876
#2  0x00005636f48c040c in bridge_topics_destroy (bridge=bridge@entry=0x7f75900b2dd0) at stasis_bridges.c:347
#3  0x00005636f479c6ce in destroy_bridge (obj=0x7f75900b2dd0) at bridge.c:666
#4  0x00005636f478c462 in __ao2_ref (user_data=0x7f75900b2dd0, delta=-1, tag=0x0, file=0x7f7772d44000 "res_stasis.c", line=1393,
   func=0x7f7772d44fb0 <__PRETTY_FUNCTION__.20350> "_dtor_last_bridge") at astobj2.c:615
#5  0x00007f7772d3861d in _dtor_last_bridge (v=<synthetic pointer>) at res_stasis.c:1545
#6  stasis_app_exec (chan=chan@entry=0x7f74f80ce6b0, app_name=<optimized out>, argc=<optimized out>, argv=argv@entry=0x7f738b928980) at res_stasis.c:1393
#7  0x00007f77728683c9 in app_exec (chan=0x7f74f80ce6b0, data=0x7f74f8080438 "agent-dial,accountCode-agentExten_1344_AD040720231307385679,timeout-30")
   at app_stasis.c:105
#8  0x00005636f486edbc in pbx_exec (c=0x7f74f80ce6b0, app=0x5636f7cdb970, data=<optimized out>)
   at /usr/src/asterisk/asterisk-certified-18.9-cert4/include/asterisk/strings.h:67
#9  0x00007f777213630d in ari_channel_thread (data=data@entry=0x7f74f8080f20) at /usr/src/asterisk/asterisk-certified-18.9-cert4/include/asterisk/strings.h:739
#10 0x00005636f48ee338 in dummy_start (data=<optimized out>) at utils.c:1572
#11 0x00007f77928df609 in start_thread (arg=<optimized out>) at pthread_create.c:477
#12 0x00007f779265f133 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
#0  __strncmp_avx2 () at ../sysdeps/x86_64/multiarch/strcmp-avx2.S:102
No locals.
{noformat}

I have fixed the issue and also have provided the patch
Comments:By: Asterisk Team (asteriskteam) 2023-04-08 07:31:03.709-0500

Thanks for creating a report! The issue has entered the triage process. That means the issue will wait in this status until a Bug Marshal has an opportunity to review the issue. Once the issue has been reviewed you will receive comments regarding the next steps towards resolution. Please note that log messages and other files should not be sent to the Sangoma Asterisk Team unless explicitly asked for. All files should be placed on this issue in a sanitized fashion as needed.

A good first step is for you to review the [Asterisk Issue Guidelines|https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines] if you haven't already. The guidelines detail what is expected from an Asterisk issue report.

Then, if you are submitting a patch, please review the [Patch Contribution Process|https://wiki.asterisk.org/wiki/display/AST/Patch+Contribution+Process].

Please note that once your issue enters an open state it has been accepted. As Asterisk is an open source project there is no guarantee or timeframe on when your issue will be looked into. If you need expedient resolution you will need to find and pay a suitable developer. Asking for an update on your issue will not yield any progress on it and will not result in a response. All updates are posted to the issue when they occur.

Please note that by submitting data, code, or documentation to Sangoma through JIRA, you accept the Terms of Use present at [https://www.asterisk.org/terms-of-use/|https://www.asterisk.org/terms-of-use/].
By: Yash Khandelwal (Yashyk456) 2023-04-08 07:38:17.769-0500

Fix issue in stasis_topic_pool_delete_topic function

The stasis_topic_pool_delete_topic function in stasis.c includes a call to the stasis_topic_name function which can return NULL. Previously, the function was being compared directly with strncmp, which caused a segmentation fault in some cases.

To fix this, I added a null check in the if statement to ensure that stasis_topic_name is not NULL before performing strncmp. This prevents the segmentation fault and ensures that the function works as intended.

This commit resolves the issue and improves the stability of the stasis_topic_pool_delete_topic function.
By: Joshua C. Colp (jcolp) 2023-04-10 05:24:31.895-0500

Until a contributor license agreement is signed, no patch can be attached. Have you identified in what cases the topic name can be NULL?
By: Yash Khandelwal (Yashyk456) 2023-04-10 10:28:25.229-0500

Have signed the contributor license agreement.

Yes , I have identified the place its a condition written in code

whenever the topic is NULL we are returning null


const char *stasis_topic_name(const struct stasis_topic *topic)
{
// this check whenever its true will face core dump
       if (!topic) {
return NULL;
}
return topic->name;
}
By: Joshua C. Colp (jcolp) 2023-04-10 10:33:49.112-0500

That doesn't really answer my question - it just changes it. Is calling code expecting the topic to be non-NULL? Is that an incorrect expectation?
By: Yash Khandelwal (Yashyk456) 2023-04-10 10:44:56.886-0500

The crash occurred while the program was trying to delete a topic from the Stasis topic pool. It seems that the topic name passed to the function stasis_topic_pool_delete_topic() was an invalid memory address , which caused the program to crash when the __strncmp_avx2() function was called to compare the topic name with the names of topics in the pool.

Basically when we get two simultaneous requests to delete the same bridge then there is a possibility .


Even if the scenario has not come  we can't compare null in strncmp. We got the case when same request came 2 time

By: Joshua C. Colp (jcolp) 2023-04-10 10:49:32.475-0500

Then that sounds like you're just working around the problem/reducing the potential race condition. If the issue is due to deleting the same bridge at the same time from two places, then that should fundamentally be fixed instead and I don't think just fixing the topic name is fixing that.
By: Yash Khandelwal (Yashyk456) 2023-04-10 10:58:59.548-0500

Yes , You are correct, that should be fundamentally correct and we have corrected that in our code also.

But I just found a condition in a code that does not looks apt to me I just came and highlighted that .
By: Yash Khandelwal (Yashyk456) 2023-04-11 09:44:19.562-0500

I am unable to access this link

https://sangoma.atlassian.net/browse/ASTDEV-175


Can you please tell me if is there something to which I can contribute, I have provided the patch already anything else I can help with
By: Joshua C. Colp (jcolp) 2023-04-11 09:48:58.263-0500

That's an internal planning link. Until legal accepts your license agreement, there is nothing to do.
By: Yash Khandelwal (Yashyk456) 2023-04-24 07:02:24.540-0500

Any update on this ?

By: Joshua C. Colp (jcolp) 2023-04-24 07:05:29.049-0500

If there are any updates they will be posted here.