summaryrefslogtreecommitdiffstats
path: root/src/gb
diff options
context:
space:
mode:
authorNeels Hofmeyr <neels@hofmeyr.de>2017-11-16 22:32:36 +0100
committerNeels Hofmeyr <neels@hofmeyr.de>2017-11-20 17:22:42 +0100
commitcd325efae564384c74b4af6163303ddc81c7a3c1 (patch)
tree91fd7a58c09d257546f188e8d9ce43933551adc2 /src/gb
parent0128c78ffe25196f53fbbc0884a9c4587f493224 (diff)
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
Diffstat (limited to 'src/gb')
-rw-r--r--src/gb/gprs_bssgp.c6
1 files changed, 5 insertions, 1 deletions
diff --git a/src/gb/gprs_bssgp.c b/src/gb/gprs_bssgp.c
index 520868e0..d27a94f7 100644
--- a/src/gb/gprs_bssgp.c
+++ b/src/gb/gprs_bssgp.c
@@ -764,7 +764,11 @@ int bssgp_fc_in(struct bssgp_flow_control *fc, struct msgb *msg,
}
if (bssgp_fc_needs_queueing(fc, llc_pdu_len)) {
- return fc_enqueue(fc, msg, llc_pdu_len, priv);
+ int rc;
+ rc = fc_enqueue(fc, msg, llc_pdu_len, priv);
+ if (rc)
+ msgb_free(msg);
+ return rc;
} else {
/* record the time we transmitted this PDU */
osmo_gettimeofday(&time_now, NULL);