From cd325efae564384c74b4af6163303ddc81c7a3c1 Mon Sep 17 00:00:00 2001 From: Neels Hofmeyr Date: Thu, 16 Nov 2017 22:32:36 +0100 Subject: gprs_bssgp: bssgp_fc_in(): fix mem leak on queue overflow All successful and all error code paths of bssgp_fc_in() free the msgb, except the code path calling fc_enqueue() when the msg is dropped (due to queue being full, or failure to allocate). Callers could theoretically catch the -ENOSPC return value and discard the msgb. However, in other code paths, a callback's return value is returned, which is expected to free the msgb, so such callback would have to never return -ENOSPC when it freed the msgb. Much simpler semantics would be to free the msgb in every code path, no matter which kind of error occurred. Who is currently calling bssgp_fc_in and how do they handle the return value? - bssgp_fc_test.c ignores the return value (and hits a mem leak aka sanitizer build failure if the queue is full). - fc_timer_cb() ignores the return value. - bssgp_tx_dl_ud() returns the bssgp_fc_in() rc. - which is returned by a cascade of functions leading up to being returned, for example, by gprs_llgmm_reset(), which is usually called with ignored return code. At this point it is already fairly clear that bssgp_fc_in() should always free the msgb, since the callers don't seem to distinguish even between error or success, let alone between -ENOSPC or other errors. bssgp_fc_test: assert that no msgbs remain unfreed after the tests. Adjust expected results. Helps fix sanitizer build on debian 9. Change-Id: I00c62a104baeaad6a85883c380259c469aebf0df --- tests/gb/bssgp_fc_tests.ok | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'tests/gb/bssgp_fc_tests.ok') diff --git a/tests/gb/bssgp_fc_tests.ok b/tests/gb/bssgp_fc_tests.ok index d1468434..30d9776e 100644 --- a/tests/gb/bssgp_fc_tests.ok +++ b/tests/gb/bssgp_fc_tests.ok @@ -56,7 +56,7 @@ size-max=100 oct, leak-rate=100 oct/s, queue-len=5 msgs, pdu_len=10 oct, pdu_cnt 30: FC OUT Nr 13 40: FC OUT Nr 14 50: FC OUT Nr 15 -msgb ctx: 685 b in 6 blocks (0 b in 1 block == just the context) +msgb ctx: 0 b in 1 blocks (0 b in 1 block == just the context) ===== BSSGP flow-control test END ===== BSSGP flow-control test START @@ -229,6 +229,6 @@ size-max=100 oct, leak-rate=100 oct/s, queue-len=5 msgs, pdu_len=10 oct, pdu_cnt 30: FC OUT Nr 13 40: FC OUT Nr 14 50: FC OUT Nr 15 -msgb ctx: 685 b in 6 blocks (0 b in 1 block == just the context) +msgb ctx: 0 b in 1 blocks (0 b in 1 block == just the context) ===== BSSGP flow-control test END -- cgit v1.2.3