aboutsummaryrefslogtreecommitdiffstats
path: root/gtp
diff options
context:
space:
mode:
authorPau Espin Pedrol <pespin@sysmocom.de>2019-08-16 13:20:09 +0200
committerPau Espin Pedrol <pespin@sysmocom.de>2019-08-23 14:38:56 +0200
commit623c5b36e9453f1623bc443e04627c2c31cb2619 (patch)
tree7010a41ae11c45dfc4a3683d2b34c44aa385345f /gtp
parentaab47afe58124981f7391a4df43821cf2f95725f (diff)
libgtp: Remove packets in tx queue belonging pdp being freed
Doing so should avoid the crash seen in OS#3956, where a message is received in osmo-sgsn gtp iface after having received a DeleteCtxAccept message where pdp and associated cbp is freed. As a result, when new confirmation arrives, it can still be matched against an old request and be sent to upper layers providing an already freed cbp. With this patch, since all queued messages belonging to that pdp are dropped, confirmation won't find a match and be discarded in libgtp. In order to be able to drop all req messages belonging to a pdp, a new list is added to pdp_t and qmsg_t are added to that list when inserted into the per-gsn req transmit queue. This way upon pdp free time it's simply a matter of iterating over that list to remove all messages. There's no need to do same for resp queue, and it'd be actually counter-productive, because it wouldn't be possible to detect and discard duplicates anymore after pdp ctx has been freed. Related: OS#3956 Change-Id: Id86d0b241454d3ad49c64c28087fd2710fa2d17a
Diffstat (limited to 'gtp')
-rw-r--r--gtp/gtp.c5
-rw-r--r--gtp/pdp.c13
-rw-r--r--gtp/pdp.h3
-rw-r--r--gtp/queue.c3
-rw-r--r--gtp/queue.h3
5 files changed, 26 insertions, 1 deletions
diff --git a/gtp/gtp.c b/gtp/gtp.c
index 94c3245..799f8a7 100644
--- a/gtp/gtp.c
+++ b/gtp/gtp.c
@@ -513,6 +513,8 @@ static int gtp_req(struct gsn_t *gsn, uint8_t version, struct pdp_t *pdp,
qmsg->cbp = cbp;
qmsg->type = ntoh8(packet->gtp0.h.type);
qmsg->fd = fd;
+ if (pdp) /* echo requests are not pdp-bound */
+ llist_add(&qmsg->entry, &pdp->qmsg_list_req);
}
gsn->seq_next++; /* Count up this time */
return 0;
@@ -697,6 +699,9 @@ static int gtp_resp(uint8_t version, struct gsn_t *gsn, struct pdp_t *pdp,
qmsg->cbp = NULL;
qmsg->type = 0;
qmsg->fd = fd;
+ /* No need to add to pdp list here, because even on pdp ctx free
+ we want to leave messages in queue_resp until timeout to
+ detect duplicates */
}
return 0;
}
diff --git a/gtp/pdp.c b/gtp/pdp.c
index d745916..eaef545 100644
--- a/gtp/pdp.c
+++ b/gtp/pdp.c
@@ -31,6 +31,7 @@
#include "pdp.h"
#include "gtp.h"
#include "lookupa.h"
+#include "queue.h"
/* ***********************************************************
* Functions related to PDP storage
@@ -156,7 +157,7 @@ int gtp_pdp_newpdp(struct gsn_t *gsn, struct pdp_t **pdp, uint64_t imsi, uint8_t
}
/* Default: Generate G-PDU sequence numbers on Tx */
(*pdp)->tx_gpdu_seq = true;
-
+ INIT_LLIST_HEAD(&(*pdp)->qmsg_list_req);
return 0;
}
}
@@ -165,7 +166,17 @@ int gtp_pdp_newpdp(struct gsn_t *gsn, struct pdp_t **pdp, uint64_t imsi, uint8_t
int pdp_freepdp(struct pdp_t *pdp)
{
+ struct qmsg_t *qmsg, *qmsg2;
struct pdp_t *pdpa = pdp->gsn->pdpa;
+ int rc;
+
+ /* Remove all enqueued messages belonging to this pdp from req tx transmit
+ queue. queue_freemsg will call llist_del(). */
+ llist_for_each_entry_safe(qmsg, qmsg2, &pdp->qmsg_list_req, entry) {
+ if ((rc = queue_freemsg(pdp->gsn->queue_req, qmsg)))
+ LOGP(DLGTP, LOGL_ERROR,
+ "Failed freeing qmsg from qmsg_list_req during pdp_freepdp()! %d\n", rc);
+ }
pdp_tiddel(pdp);
diff --git a/gtp/pdp.h b/gtp/pdp.h
index d64d394..fdfa824 100644
--- a/gtp/pdp.h
+++ b/gtp/pdp.h
@@ -17,6 +17,7 @@
#include <netinet/in.h>
#include <osmocom/core/defs.h>
+#include <osmocom/core/linuxlist.h>
struct gsn_t;
@@ -241,6 +242,8 @@ struct pdp_t {
struct gsn_t *gsn; /* Back pointer to GSN where this pdp ctx belongs to */
bool tx_gpdu_seq; /* Transmit (true) or suppress G-PDU sequence numbers */
+
+ struct llist_head qmsg_list_req; /* list of req qmsg_t in retrans queue belonging this pdp ctx */
};
/* functions related to pdp_t management */
diff --git a/gtp/queue.c b/gtp/queue.c
index ce4713e..4c25913 100644
--- a/gtp/queue.c
+++ b/gtp/queue.c
@@ -172,6 +172,7 @@ int queue_newmsg(struct queue_t *queue, struct qmsg_t **qmsg,
} else {
*qmsg = &queue->qmsga[queue->next];
queue_seqset(queue, *qmsg, peer, seq);
+ INIT_LLIST_HEAD(&(*qmsg)->entry);
(*qmsg)->state = 1; /* Space taken */
(*qmsg)->this = queue->next;
(*qmsg)->next = -1; /* End of the queue */
@@ -206,6 +207,8 @@ int queue_freemsg(struct queue_t *queue, struct qmsg_t *qmsg)
return EOF; /* Not in queue */
}
+ llist_del(&qmsg->entry);
+
queue_seqdel(queue, qmsg);
if (qmsg->next == -1) /* Are we the last in queue? */
diff --git a/gtp/queue.h b/gtp/queue.h
index 76cb7be..9b0367b 100644
--- a/gtp/queue.h
+++ b/gtp/queue.h
@@ -17,6 +17,8 @@
#ifndef _QUEUE_H
#define _QUEUE_H
+#include <osmocom/core/linuxlist.h>
+
#include "gtp.h"
#define QUEUE_DEBUG 0 /* Print debug information */
@@ -39,6 +41,7 @@ struct qmsg_t { /* Holder for queued packets */
int this; /* Pointer to myself */
time_t timeout; /* When do we retransmit this packet? */
int retrans; /* How many times did we retransmit this? */
+ struct llist_head entry; /* Listed with other qmsg_t belonging to a pdp_t->qmsg_list_req */
};
struct queue_t {