Skip to content

Lockless changes for CSFB procedure, MO/MT call and SMS #1

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

Open
wants to merge 42 commits into
base: rsarwad_merge_csfb
Choose a base branch
from

Conversation

rsarwad
Copy link
Owner

@rsarwad rsarwad commented Jan 8, 2020

Summary: CSFB Lock less changes for MO/MT call and SMS, MM information, Non EPS Alert procedure and UE activity Indication procedure
Type: Refactor
Test Plan: Executed s1ap tester test suite

/* If ECM state is IDLE send
* service_reject in Establish cnf else send in DL NAS Transport
*/
rc = emm_proc_service_reject(ue_id,
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use mme_app_notify_service_reject_to_nas () and pass appropriate arguments

/* send Service Reject to UE */
mme_app_notify_service_reject_to_nas(ue_context_p->mme_ue_s1ap_id, EMM_CAUSE_CONGESTION,
UE_CONTEXT_MODIFICATION_PROCEDURE_FAILED);
/* Notify Service Reject to NAS, which shall Service Reject message
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notify nas module to send Service Reject to UE

@@ -2427,34 +2447,34 @@ int handle_csfb_s1ap_procedure_failure(
** Return: RETURNok, RETURNerror **
** **
***************************************************************************/

int mme_app_notify_service_reject_to_nas(
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Return void and log message.
Make sure the caller of this function has appropriate logs

** inputs: ue_context_p: Pointer to UE context **
** emm_casue: failed cause **
** **
** outputs: **
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove outputs

} break;

case SGSAP_MM_INFORMATION_REQ: {
/*Received SGSAP MM Information Request message from SGS task*/
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Single line comment

@rsarwad rsarwad force-pushed the rsarwad_merge_csfb branch from 67dd562 to ec6d70a Compare January 14, 2020 02:49
@rsarwad rsarwad force-pushed the rsarwad_merge_csfb branch from de84313 to e15490c Compare January 14, 2020 04:41
alexsn and others added 10 commits January 14, 2020 00:02
Reviewed By: idoshveki

Differential Revision: D19377014

fbshipit-source-id: f2e8c0db1254ae5ce4ff54be13c7c2918dfff46c
Reviewed By: hanle11

Differential Revision: D19373214

fbshipit-source-id: 13d167ea59cd2d8a368dc9405b1f17c7549fe364
Summary: Currently only supports being shown above, and doesn't handle the screen's borders.

Reviewed By: rckclmbr

Differential Revision: D19329857

fbshipit-source-id: bdf2ed6cca0b4e8c5635e38cd2e49dfdde4c8dd0
Summary:
Pull Request resolved: https://github.com/facebookincubator/symphony/pull/213

implement bounds constraint for Ints

Reviewed By: idoshveki

Differential Revision: D19371726

fbshipit-source-id: 68254a6a3f9c232e5b1be08416c3d1762edad955
Summary:
Pull Request resolved: https://github.com/facebookincubator/symphony/pull/211

Adding locations+positions to links export (to support import - next diff)
fixed tests  + skipped tests that includes import.

Reviewed By: alexsn

Differential Revision: D19364394

fbshipit-source-id: 023650736dcf78314b5516be68c379aa55a80aa4
Summary:
Pull Request resolved: https://github.com/facebookincubator/symphony/pull/214

- After changing the export structure - modifying the import respectively.
   - Adding new entity "portInLink" to simplify the access and line validations of each port in a link (no other entity has two locations and positions hierarchies in it).

- Fixing relevant tests

Reviewed By: a8m

Differential Revision: D19373254

fbshipit-source-id: 3fe1416ee38a99fd6039c72e14eed15a8d75c854
Summary: Add a field to record when the survey was first started and a field to track the status.

Reviewed By: dalmendray, Scott8440

Differential Revision: D19341010

fbshipit-source-id: 5ae59a2fa96bd0bcbb1d2e52d1195b89171591c7
…st fixes & outstanding pulls (magma#1091)

Summary:
Pull Request resolved: magma#1091

It'll allow fast turnaround yet, still use github as source for go-diameter

Reviewed By: mpgermano

Differential Revision: D19386844

fbshipit-source-id: 20e9070b9e3661507a9663177aafba06d7bcf2d7
…a#1087)

Summary:
Pull Request resolved: magma#1087

Not dealing with hosting the interactive UI at all, the docs themselves should be useful enough. Will make developing on these products much easier. Also will be useful when I begin versioning and redesigning the API for v1.

Reviewed By: aclave1

Differential Revision: D19377230

fbshipit-source-id: 019300bacacd2d68b5b57d33578d3d1b9ae2e56e
…d detach procedure (magma#1035)

Summary:
Lock less changes for CSFB related changes for combined attach, TAU and detach procedure
Type: Refactor
Pull Request resolved: magma#1035

Test Plan: Executed s1ap tester test suite

Reviewed By: ardzoht

Differential Revision: D19393873

Pulled By: ssanadhya

fbshipit-source-id: 36ff5bff75f040c3fb6cede0454d40d7bd55b29a
tcirstea and others added 4 commits January 14, 2020 13:40
Summary:
Figured out the dependencies of `serialize-javascript`: `webpack-terser`.

Update it using `yarn upgrade-interactive`.

Look at `yarn.lock` to ensure that version of `serialize-javascript` is not vulnerable.
Realize it still is.

Merge a bunch of things manually that were split up. Mainly `terser-webpack-plugin@^1.2.3` with `terser-webpack-plugin@^1.2.4`, run `yarn install`.

Pull Request resolved: facebookexternal/terragraph-nms#267

Pull Request resolved: magma#1085

Pull Request resolved: https://github.com/facebookincubator/symphony/pull/212

Pull Request resolved: facebookexternal/fbc#1817

Reviewed By: Scott8440

Differential Revision: D19375744

fbshipit-source-id: 9e3ff07caee9320e4fc05ec09722423a7c7cb76c
Summary:
Pull Request resolved: magma#1088

These are independent services which don't affect the same files which means there's no reason to have them in the same container. Makes deployment simpler and more transparent.
Also use `alpine` base image instead of ubuntu, decreasing size of images from `130MB` to `18.7MB`

Reviewed By: xjtian

Differential Revision: D19355844

fbshipit-source-id: 3b58dd45e57195185a49fb284f3b09373e4c8b67
…ipts (magma#1086)

Summary:
Pull Request resolved: magma#1086

* Deploy `alertmanager-configurer` and `prometheus-configurer` as two separate containers via Helm.
* Update deployment READMEs to reflect this change
* Update deployment scripts to publish the correct containers

Reviewed By: themarwhal, xjtian

Differential Revision: D19375773

fbshipit-source-id: 5c244dc4ca0767431a3db487210bc78ee0d34518
Summary: Pass appropriate error code from AAA service in responce to ASRs

Differential Revision: D19358902

fbshipit-source-id: 262b5f73ebce84814fede3885eab017418c78c2a
@rsarwad rsarwad force-pushed the rsarwad_merge_csfb_call_sms branch 2 times, most recently from 81fbda1 to 66b5e85 Compare January 15, 2020 13:42
alexsn and others added 11 commits January 15, 2020 06:57
)

Summary:
Pull Request resolved: https://github.com/facebookincubator/symphony/pull/217

inventory ui assumes edges are not nullable even though we define them as such in graphql schema. updating js code is too much work compared to this alternative.

Reviewed By: dlvhdr

Differential Revision: D19409585

fbshipit-source-id: 3dcb0c2b50696d68f9e958cd6dbbf9e819838bd4
Summary: Filters weren't removed if they previously had a value and then the user deleted the value.

Reviewed By: AmitArbel

Differential Revision: D19373314

fbshipit-source-id: a6da52d44006a4b299966a1f56e8edcfffad35a7
Summary:
Had a bug where if you had a location filter in the equipment search, clicked next and then back, the wizard would crash.
This happened because the filter config for the existing value wasn't present.

Note: we probably need to refactor the search in overall, as it's pretty flakey and the flow types aren't the greatest.

Reviewed By: AmitArbel

Differential Revision: D19390967

fbshipit-source-id: 844164575ee7357a9fd690ca5b79933b18d8f272
Summary: Couldn't delete location after removing all its equipment until you refresh the page.

Reviewed By: AmitArbel

Differential Revision: D19364014

fbshipit-source-id: a9b24ff7fa2df008cf905872a4fae4cb5f1cfcbe
Summary: Pull Request resolved: https://github.com/facebookincubator/symphony/pull/215

Reviewed By: alexsn

Differential Revision: D19390935

fbshipit-source-id: fd8c66d6c9c845a61da8694e9985486491591e4e
Summary:
Last command in a composite write command to the device was being cached and subsequent
last commands of other composite write commands have not been executed on the device
Pull Request resolved: magma#1023

Differential Revision: D19411381

Pulled By: Mokon

fbshipit-source-id: f811fbecd0f15a1b27cba9995841f086b7f33fc8
Summary:
This diff adds a dockerfile for the uesimulator service.
This file will also work for any future golang services at
`magma/cwf/gateway/...`. Having the ability to run the uesimulator
service as a docker container will be useful for both the CWF
integration test system (e.g. simulating 2 WACs) and FeG HA chaos
testing.

Reviewed By: xjtian

Differential Revision: D19295230

fbshipit-source-id: 00117b14626c4234d1a60b673eebee0118c1e99c
Summary:
The `third-party` directory was recently removed from
the FeG but this `uesim` dockerfile wasn't updated. This broke
our integration tests. This diff fixes that.

Reviewed By: themarwhal

Differential Revision: D19417803

fbshipit-source-id: 15156e773062abfa1a9479cba91d2bf96ad77467
Summary: Handling the ResultCode for MSCC in Gy.

Reviewed By: themarwhal

Differential Revision: D19406790

fbshipit-source-id: a73e65d02b593b69cc642d09a2bce079eb888626
@rsarwad rsarwad force-pushed the rsarwad_merge_csfb_call_sms branch from 66b5e85 to 93be6ec Compare January 16, 2020 03:07
rsarwad pushed a commit that referenced this pull request May 12, 2020
Summary:
Pull Request resolved: facebookexternal/magma-distribution#1

Pull Request resolved: magma#1633

Pull Request resolved: facebookexternal/fbc#2397

Reviewed By: rckclmbr

Differential Revision: D21456981

fbshipit-source-id: 8023312063f87669d20d30416a1cadf81b019af0
rsarwad pushed a commit that referenced this pull request Jun 18, 2020
Summary:
Pull Request resolved: magma#1818

Running valgrind on sessiond and running `make integ_test TESTS=s1aptests/test_attach_detach_multi_ue.py`
(Also the memory leak observed was scaling with the number of UEs so it was definitely an actual issue)

Before the change
```==5861==
==5861== 14,912 (3,840 direct, 11,072 indirect) bytes in 32 blocks are definitely lost in loss record 1,727 of 1,729
==5861==    at 0x4C2C21F: operator new(unsigned long) (vg_replace_malloc.c:334)
==5861==    by 0x6E6342: magma::SessionState::is_dynamic_rule_installed(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (SessionState.cpp:486)
==5861==    by 0x745780: magma::lte::SessionStore::merge_into_session(std::unique_ptr<magma::SessionState, std::default_delete<magma::SessionState> >&, magma::SessionStateUpdateCriteria&) (SessionStore.cpp:213)
==5861==    by 0x744201: magma::lte::SessionStore::update_sessions(std::unordered_map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::unordered_map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, magma::SessionStateUpdateCriteria, std::hash<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::equal_to<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, magma::SessionStateUpdateCriteria> > >, std::hash<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::equal_to<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::unordered_map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, magma::SessionStateUpdateCriteria, std::hash<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::equal_to<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, magma::SessionStateUpdateCriteria> > > > > > const&) (SessionStore.cpp:104)
==5861==    by 0x693858: magma::LocalSessionManagerHandlerImpl::end_session(std::unordered_map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::vector<std::unique_ptr<magma::SessionState, std::default_delete<magma::SessionState> >, std::allocator<std::unique_ptr<magma::SessionState, std::default_delete<magma::SessionState> > > >, std::hash<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::equal_to<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::vector<std::unique_ptr<magma::SessionState, std::default_delete<magma::SessionState> >, std::allocator<std::unique_ptr<magma::SessionState, std::default_delete<magma::SessionState> > > > > > >&, magma::lte::LocalEndSessionRequest const&, std::function<void (grpc::Status, magma::lte::LocalEndSessionResponse)>) (LocalSessionManagerHandler.cpp:445)
==5861==    by 0x6935AA: magma::LocalSessionManagerHandlerImpl::EndSession(grpc::ServerContext*, magma::lte::LocalEndSessionRequest const*, std::function<void (grpc::Status, magma::lte::LocalEndSessionResponse)>)::{lambda()#1}::operator()() const (LocalSessionManagerHandler.cpp:433)
==5861==    by 0x695BD1: void folly::detail::function::FunctionTraits<void ()>::callBig<magma::LocalSessionManagerHandlerImpl::EndSession(grpc::ServerContext*, magma::lte::LocalEndSessionRequest const*, std::function<void (grpc::Status, magma::lte::LocalEndSessionResponse)>)::{lambda()#1}>(folly::detail::function::Data&) (Function.h:321)
==5861==    by 0x54BDF8D: folly::NotificationQueue<folly::Function<void ()> >::Consumer::consumeMessages(bool, unsigned long*) (in /usr/local/lib/libfolly.so.57.0.0)
==5861==    by 0x97AD59F: event_base_loop (in /usr/lib/x86_64-linux-gnu/libevent-2.0.so.5.1.9)
==5861==    by 0x54B8D2F: folly::EventBase::loopBody(int) (in /usr/local/lib/libfolly.so.57.0.0)
==5861==    by 0x54BA375: folly::EventBase::loopForever() (in /usr/local/lib/libfolly.so.57.0.0)
==5861==    by 0x6AC968: magma::LocalEnforcer::start() (LocalEnforcer.cpp:125)
==5861==
==5861== LEAK SUMMARY:
==5861==    definitely lost: 7,680 bytes in 64 blocks
==5861==    indirectly lost: 11,072 bytes in 224 blocks
==5861==      possibly lost: 43,919 bytes in 100 blocks
==5861==    still reachable: 528,915 bytes in 7,075 blocks
==5861==                       of which reachable via heuristic:
==5861==                         stdstring          : 181 bytes in 2 blocks
==5861==                         multipleinheritance: 3,090 bytes in 10 blocks
==5861==         suppressed: 0 bytes in 0 blocks
==5861== Reachable blocks (those to which a pointer was found) are not shown.
==5861== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==5861==
==5861== For counts of detected and suppressed errors, rerun with: -v
==5861== ERROR SUMMARY: 71 errors from 71 contexts (suppressed: 0 from 0)
vagrant@magma-dev:~/magma/lte/gateway$
```

After the change
```==10143==
==10143== LEAK SUMMARY:
==10143==    definitely lost: 0 bytes in 0 blocks
==10143==    indirectly lost: 0 bytes in 0 blocks
==10143==      possibly lost: 39,878 bytes in 97 blocks
==10143==    still reachable: 529,283 bytes in 7,078 blocks
==10143==                       of which reachable via heuristic:
==10143==                         stdstring          : 181 bytes in 2 blocks
==10143==                         multipleinheritance: 3,090 bytes in 10 blocks
==10143==         suppressed: 0 bytes in 0 blocks
==10143== Reachable blocks (those to which a pointer was found) are not shown.
==10143== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==10143==
==10143== For counts of detected and suppressed errors, rerun with: -v
==10143== ERROR SUMMARY: 67 errors from 67 contexts (suppressed: 0 from 0)
```

Reviewed By: karthiksubraveti

Differential Revision: D22042230

fbshipit-source-id: 77e051149a54bbed2c8ec746405c5f75b6632489
rsarwad pushed a commit that referenced this pull request Jun 18, 2020
Summary:
ASAN error:
```
Jun 16 08:56:03 magma-dev sessiond[9856]: =================================================================
Jun 16 08:56:03 magma-dev sessiond[9856]: ==9856==ERROR: AddressSanitizer: new-delete-type-mismatch on 0x61800001fc80 in thread T14:
Jun 16 08:56:03 magma-dev sessiond[9856]:   object passed to delete has wrong type:
Jun 16 08:56:03 magma-dev sessiond[9856]:   size of the allocated type:   816 bytes;
Jun 16 08:56:03 magma-dev sessiond[9856]:   size of the deallocated type: 808 bytes.
Jun 16 08:56:03 magma-dev sessiond[9856]: I0616 08:56:03.149473  9886 SessionEvents.cpp:53] Could not log session_created event {"session_id":"IMSI001010000000001-120251","imsi":"IMSI001010000000001"}, Error Message: Connect Failed
Jun 16 08:56:03 magma-dev sessiond[9856]:     #0 0x7ff448b0d7f0 in operator delete(void*, unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc37f0)
Jun 16 08:56:03 magma-dev sessiond[9856]:     #1 0x559f94cbc742 in magma::AsyncGRPCRequest<magma::lte::LocalSessionManager::WithAsyncMethod_ReportRuleStats<magma::lte::LocalSessionManager::WithAsyncMethod_CreateSession<magma::lte::LocalSessionManager::WithAsyncMethod_EndSession<magma::lte::LocalSessionManager::Service> > >, magma::lte::LocalCreateSessionRequest, magma::lte::LocalCreateSessionResponse>::proceed() /home/vagrant/magma/lte/gateway/c/session_manager/SessionManagerServer.cpp:98
Jun 16 08:56:03 magma-dev sessiond[9856]:     #2 0x559f94ca8f54 in magma::AsyncService::wait_for_requests() /home/vagrant/magma/lte/gateway/c/session_manager/SessionManagerServer.cpp:39
Jun 16 08:56:03 magma-dev sessiond[9856]:     #3 0x559f94c7cae9 in operator() /home/vagrant/magma/lte/gateway/c/session_manager/sessiond_main.cpp:259
Jun 16 08:56:03 magma-dev sessiond[9856]:     #4 0x559f94c81c51 in _M_invoke<> /usr/include/c++/6/functional:1391
Jun 16 08:56:03 magma-dev sessiond[9856]:     #5 0x559f94c81a70 in operator() /usr/include/c++/6/functional:1380
Jun 16 08:56:03 magma-dev sessiond[9856]:     #6 0x559f94c8192b in _M_run /usr/include/c++/6/thread:197
Jun 16 08:56:03 magma-dev sessiond[9856]:     #7 0x7ff446dbde6e  (/usr/lib/x86_64-linux-gnu/libstdc++.so.6+0xb9e6e)
Jun 16 08:56:03 magma-dev sessiond[9856]:     #8 0x7ff447d384a3 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x74a3)
Jun 16 08:56:03 magma-dev sessiond[9856]:     #9 0x7ff446532d0e in __clone (/lib/x86_64-linux-gnu/libc.so.6+0xe8d0e)
Jun 16 08:56:03 magma-dev sessiond[9856]: 0x61800001fc80 is located 0 bytes inside of 816-byte region [0x61800001fc80,0x61800001ffb0)
Jun 16 08:56:03 magma-dev sessiond[9856]: allocated by thread T14 here:
Jun 16 08:56:03 magma-dev sessiond[9856]:     #0 0x7ff448b0cbf0 in operator new(unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc2bf0)
Jun 16 08:56:03 magma-dev sessiond[9856]:     #1 0x559f94ca91f0 in magma::LocalSessionManagerAsyncService::init_call_data() /home/vagrant/magma/lte/gateway/c/session_manager/SessionManagerServer.cpp:60
```

According to the internet, "Polymorphic base classes should declare virtual destructors. If a class has any virtual functions, it should have a virtual destructor".
Resource : https://stackoverflow.com/questions/41552966/getting-new-delete-type-mismatch-from-asan

Reviewed By: uri200

Differential Revision: D22065108

fbshipit-source-id: ff146a9a92f71408ea25dc86943938ca5afe88a1
rsarwad pushed a commit that referenced this pull request Jul 17, 2020
Summary:
This is pull request was created automatically because we noticed your project was missing a Contributing file.

CONTRIBUTING files explain how a developer can contribute to the project - which you should actively encourage.

This PR was crafted with love by Facebook's Open Source Team.

Pull Request resolved: magma/fbc-js-core#1

Reviewed By: dlvhdr

Differential Revision: D22570995

Pulled By: a8m

fbshipit-source-id: 71f12cb73da5d38cf7e045d41e3ba7e6e592f17e
rsarwad pushed a commit that referenced this pull request Nov 30, 2020
* Add T3489 tests

Introduce a new test to validate T3489 expiry.

Credit to ulaskozat for the diff

Testing done:
Verified that an ASAN use after free occurs on timer expiry

=7031==ERROR: AddressSanitizer: heap-use-after-free on address 0x603000093460 at pc 0x555807545462 bp 0x7f87093fd2b0 sp 0x7f87093fd2a8
WRITE of size 8 at 0x603000093460 thread T16
    #0 0x555807545461 in nas_stop_T3489 /home/vagrant/magma/lte/gateway/c/oai/tasks/nas/esm/esm_data_context.c:101
    #1 0x5558075c47c5 in esm_proc_esm_information_response /home/vagrant/magma/lte/gateway/c/oai/tasks/nas/esm/esm_information.c:119
    #2 0x55580759339b in esm_recv_information_response /home/vagrant/magma/lte/gateway/c/oai/tasks/nas/esm/sap/esm_recv.c:575
    #3 0x555807551fba in _esm_sap_recv /home/vagrant/magma/lte/gateway/c/oai/tasks/nas/esm/sap/esm_sap.c:679
    #4 0x555807550f33 in esm_sap_send /home/vagrant/magma/lte/gateway/c/oai/tasks/nas/esm/sap/esm_sap.c:283
    #5 0x5558075195a0 in lowerlayer_data_ind /home/vagrant/magma/lte/gateway/c/oai/tasks/nas/emm/LowerLayer.c:276
    #6 0x55580757848f in _emm_as_data_ind /home/vagrant/magma/lte/gateway/c/oai/tasks/nas/emm/sap/emm_as.c:688
    #7 0x555807574ec4 in emm_as_send /home/vagrant/magma/lte/gateway/c/oai/tasks/nas/emm/sap/emm_as.c:180
    #8 0x55580753147f in emm_sap_send /home/vagrant/magma/lte/gateway/c/oai/tasks/nas/emm/sap/emm_sap.c:105
    #9 0x5558074d74fc in nas_proc_ul_transfer_ind /home/vagrant/magma/lte/gateway/c/oai/tasks/nas/nas_proc.c:326
    #10 0x5558071bd634 in handle_message /home/vagrant/magma/lte/gateway/c/oai/tasks/mme_app/mme_app_main.c:97
    #11 0x7f871bb277bd in zloop_start (/usr/lib/x86_64-linux-gnu/libczmq.so.4+0x287bd)
    #12 0x5558071bf169 in mme_app_thread /home/vagrant/magma/lte/gateway/c/oai/tasks/mme_app/mme_app_main.c:447
    #13 0x7f871e11f4a3 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x74a3)
    #14 0x7f871a494d0e in __clone (/lib/x86_64-linux-gnu/libc.so.6+0xe8d0e)

0x603000093460 is located 0 bytes inside of 32-byte region [0x603000093460,0x603000093480)
freed by thread T16 here:
    #0 0x7f871e602a10 in free (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc1a10)
    #1 0x5558070dc054 in free_wrapper /home/vagrant/magma/lte/gateway/c/oai/common/dynamic_memory_check.c:47
    #2 0x555807545496 in nas_stop_T3489 /home/vagrant/magma/lte/gateway/c/oai/tasks/nas/esm/esm_data_context.c:103
    #3 0x5558075c517a in _esm_information /home/vagrant/magma/lte/gateway/c/oai/tasks/nas/esm/esm_information.c:269
    #4 0x5558075c4e15 in _esm_information_t3489_handler /home/vagrant/magma/lte/gateway/c/oai/tasks/nas/esm/esm_information.c:199
    #5 0x5558074e2e8a in mme_app_nas_timer_handle_signal_expiry /home/vagrant/magma/lte/gateway/c/oai/tasks/nas/util/nas_timer.c:100
    #6 0x5558071be2d2 in handle_message /home/vagrant/magma/lte/gateway/c/oai/tasks/mme_app/mme_app_main.c:235
    #7 0x7f871bb277bd in zloop_start (/usr/lib/x86_64-linux-gnu/libczmq.so.4+0x287bd)

Signed-off-by: Amar Padmanabhan <[email protected]>

* Invalidate the T3849 timer id while processing esm information retransmit

The _esm_information function stops the existing T3849 timer as referenced
by the esm_ctxt datastructure timer before rescheduling a new T3849 timer
when it requests for the esm info from a UE.
Stopping the timer has a side effect of freeing up the UE related
retransmission data associated with it. This causes issues during
the T3849 timer expiry handling as the cancelled timer and the rescheduled
one reuse the same retransmission data datastructure.

Fix this by unsetting the T3849 timer in the handling of the timer expiry
as the esm_ctxt is not associated with any valid timers anymore. Further
as the timer is a oneshot timer it will be cleaned up after the processing
of the timer callback.

Signed-off-by: Amar Padmanabhan <[email protected]>
rsarwad pushed a commit that referenced this pull request Apr 19, 2021
* Add T3489 tests

Introduce a new test to validate T3489 expiry.

Credit to ulaskozat for the diff

Testing done:
Verified that an ASAN use after free occurs on timer expiry

=7031==ERROR: AddressSanitizer: heap-use-after-free on address 0x603000093460 at pc 0x555807545462 bp 0x7f87093fd2b0 sp 0x7f87093fd2a8
WRITE of size 8 at 0x603000093460 thread T16
    #0 0x555807545461 in nas_stop_T3489 /home/vagrant/magma/lte/gateway/c/oai/tasks/nas/esm/esm_data_context.c:101
    #1 0x5558075c47c5 in esm_proc_esm_information_response /home/vagrant/magma/lte/gateway/c/oai/tasks/nas/esm/esm_information.c:119
    #2 0x55580759339b in esm_recv_information_response /home/vagrant/magma/lte/gateway/c/oai/tasks/nas/esm/sap/esm_recv.c:575
    #3 0x555807551fba in _esm_sap_recv /home/vagrant/magma/lte/gateway/c/oai/tasks/nas/esm/sap/esm_sap.c:679
    #4 0x555807550f33 in esm_sap_send /home/vagrant/magma/lte/gateway/c/oai/tasks/nas/esm/sap/esm_sap.c:283
    #5 0x5558075195a0 in lowerlayer_data_ind /home/vagrant/magma/lte/gateway/c/oai/tasks/nas/emm/LowerLayer.c:276
    #6 0x55580757848f in _emm_as_data_ind /home/vagrant/magma/lte/gateway/c/oai/tasks/nas/emm/sap/emm_as.c:688
    #7 0x555807574ec4 in emm_as_send /home/vagrant/magma/lte/gateway/c/oai/tasks/nas/emm/sap/emm_as.c:180
    #8 0x55580753147f in emm_sap_send /home/vagrant/magma/lte/gateway/c/oai/tasks/nas/emm/sap/emm_sap.c:105
    #9 0x5558074d74fc in nas_proc_ul_transfer_ind /home/vagrant/magma/lte/gateway/c/oai/tasks/nas/nas_proc.c:326
    #10 0x5558071bd634 in handle_message /home/vagrant/magma/lte/gateway/c/oai/tasks/mme_app/mme_app_main.c:97
    #11 0x7f871bb277bd in zloop_start (/usr/lib/x86_64-linux-gnu/libczmq.so.4+0x287bd)
    #12 0x5558071bf169 in mme_app_thread /home/vagrant/magma/lte/gateway/c/oai/tasks/mme_app/mme_app_main.c:447
    #13 0x7f871e11f4a3 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x74a3)
    #14 0x7f871a494d0e in __clone (/lib/x86_64-linux-gnu/libc.so.6+0xe8d0e)

0x603000093460 is located 0 bytes inside of 32-byte region [0x603000093460,0x603000093480)
freed by thread T16 here:
    #0 0x7f871e602a10 in free (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc1a10)
    #1 0x5558070dc054 in free_wrapper /home/vagrant/magma/lte/gateway/c/oai/common/dynamic_memory_check.c:47
    #2 0x555807545496 in nas_stop_T3489 /home/vagrant/magma/lte/gateway/c/oai/tasks/nas/esm/esm_data_context.c:103
    #3 0x5558075c517a in _esm_information /home/vagrant/magma/lte/gateway/c/oai/tasks/nas/esm/esm_information.c:269
    #4 0x5558075c4e15 in _esm_information_t3489_handler /home/vagrant/magma/lte/gateway/c/oai/tasks/nas/esm/esm_information.c:199
    #5 0x5558074e2e8a in mme_app_nas_timer_handle_signal_expiry /home/vagrant/magma/lte/gateway/c/oai/tasks/nas/util/nas_timer.c:100
    #6 0x5558071be2d2 in handle_message /home/vagrant/magma/lte/gateway/c/oai/tasks/mme_app/mme_app_main.c:235
    #7 0x7f871bb277bd in zloop_start (/usr/lib/x86_64-linux-gnu/libczmq.so.4+0x287bd)

Signed-off-by: Amar Padmanabhan <[email protected]>

* Invalidate the T3849 timer id while processing esm information retransmit

The _esm_information function stops the existing T3849 timer as referenced
by the esm_ctxt datastructure timer before rescheduling a new T3849 timer
when it requests for the esm info from a UE.
Stopping the timer has a side effect of freeing up the UE related
retransmission data associated with it. This causes issues during
the T3849 timer expiry handling as the cancelled timer and the rescheduled
one reuse the same retransmission data datastructure.

Fix this by unsetting the T3849 timer in the handling of the timer expiry
as the esm_ctxt is not associated with any valid timers anymore. Further
as the timer is a oneshot timer it will be cleaned up after the processing
of the timer callback.

Signed-off-by: Amar Padmanabhan <[email protected]>
rsarwad pushed a commit that referenced this pull request Jun 16, 2021
* Create a superbuild for Li-agent

Move files into a sub directory

Signed-off-by: Amar Padmanabhan <[email protected]>

* Cleanup submakefile for super build

- Include package dependencies from common
- Add dependency on MAGMA_LOGGING package
- Add dependency on mobilityd grpc, which in turns brings in subscriberdb
which brings in apn
- Remove all include directory directives.

Signed-off-by: Amar Padmanabhan <[email protected]>

* Update makefile and deploy script

Update makefile and deploy script

Signed-off-by: Amar Padmanabhan <[email protected]>

* Move test directory

Move test directory to new location under superbuild


Signed-off-by: Amar Padmanabhan <[email protected]>

* Update li_agent tests for new superbuild

Replace include_directory with target_include_directory
Update Makefile target

Testing done:
1/1 Test #1: test_pdu_generator ...............   Passed    0.02 sec

100% tests passed, 0 tests failed out of 1

Signed-off-by: Amar Padmanabhan <[email protected]>

Co-authored-by: Amar Padmanabhan <[email protected]>
rsarwad pushed a commit that referenced this pull request Mar 7, 2022
Addresses one finding (more exist) of magma#11826.

Zero-initialized all instances of `plmn_array[PLMN_BYTES]` (so that they will be null terminated) and enlarged the array by one char to accommodate the null termination.

Fixes the finding:

```
[ RUN      ] TestAMFStateConverter.TestUEm5gmmContextToProto
=================================================================
==15482==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffee811fc86 at pc 0x7f3038dada6d bp 0x7ffee811faa0 sp 0x7ffee811f248
READ of size 7 at 0x7ffee811fc86 thread T0
    #0 0x7f3038dada6c  (/lib/x86_64-linux-gnu/libasan.so.5+0x67a6c)
    #1 0x7f302e641e9b in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(char const*, std::allocator<char> const&) (/lib/x86_64-linux-gnu/libstdc++.so.6+0x145e9b)
    #2 0x7f30383b85f6 in magma::lte::oai::Tai::set_mcc_mnc(char const*) bazel-out/k8-dbg/bin/lte/protos/oai/nas_state_cpp_proto_pb/lte/protos/oai/nas_state.pb.h:11239
```

## Test Plan

Using prototype Bazel build with `--config=asan` validated ASAN finding
is resolved.

Signed-off-by: Scott Moeller <[email protected]>
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.

9 participants