[PATCH 1/3] gtphub: fix gtphub_read() semantics.
gtphub always wants to know the sender, hence make the from_addr pointer mandatory. Fixes two coverity complaints (1339766, 1339764). Sponsored-by: On-Waves ehi --- openbsc/src/gprs/gtphub.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/openbsc/src/gprs/gtphub.c b/openbsc/src/gprs/gtphub.c index e00d6cd..049e9f5 100644 --- a/openbsc/src/gprs/gtphub.c +++ b/openbsc/src/gprs/gtphub.c @@ -784,16 +784,16 @@ static int gtphub_bind_start(struct gtphub_bind *b, return 0; } -/* Recv datagram from from->fd, optionally write sender's address to *from_addr. +/* Recv datagram from from->fd, write sender's address to *from_addr. * Return the number of bytes read, zero on error. */ static int gtphub_read(const struct osmo_fd *from, struct osmo_sockaddr *from_addr, uint8_t *buf, size_t buf_len) { - /* recvfrom requires the available length set in *from_addr_len. */ - if (from_addr) - from_addr->l = sizeof(from_addr->a); + OSMO_ASSERT(from_addr); + /* recvfrom requires the available length set in *from_addr_len. */ + from_addr->l = sizeof(from_addr->a); errno = 0; ssize_t received = recvfrom(from->fd, buf, buf_len, 0, (struct sockaddr*)&from_addr->a, @@ -807,12 +807,10 @@ static int gtphub_read(const struct osmo_fd *from, return 0; } - if (from_addr) { - LOG(LOGL_DEBUG, "from %s\n", osmo_sockaddr_to_str(from_addr)); - } + LOG(LOGL_DEBUG, "Received %d bytes from %s\n%s\n", + (int)received, osmo_sockaddr_to_str(from_addr), + osmo_hexdump(buf, received)); - LOG(LOGL_DEBUG, "Received %d %s\n", - (int)received, osmo_hexdump(buf, received)); return received; } -- 2.1.4
While reworking the logging, a stray comma found its way into the code. Fixes coverity 1339765. Sponsored-by: On-Waves ehi --- openbsc/src/gprs/gtphub.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openbsc/src/gprs/gtphub.c b/openbsc/src/gprs/gtphub.c index 049e9f5..406d60d 100644 --- a/openbsc/src/gprs/gtphub.c +++ b/openbsc/src/gprs/gtphub.c @@ -1878,7 +1878,7 @@ int gtphub_start(struct gtphub *hub, struct gtphub_cfg *cfg) &hub->to_sgsns[plane_idx], &cfg->sgsn_proxy[plane_idx]) != 0) { - LOG(LOGL_FATAL, "Cannot configure SGSN proxy", + LOG(LOGL_FATAL, "Cannot configure SGSN proxy" " %s port %d.\n", cfg->sgsn_proxy[plane_idx].addr_str, (int)cfg->sgsn_proxy[plane_idx].port); -- 2.1.4
- an unnecessary if-not-NULL check (1339764); - a missing nul termination safety net (1339768); - a typo resulting in the wrong proxy being logged (1339767). Sponsored-by: On-Waves ehi --- openbsc/src/gprs/gtphub.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/openbsc/src/gprs/gtphub.c b/openbsc/src/gprs/gtphub.c index 406d60d..22da9a3 100644 --- a/openbsc/src/gprs/gtphub.c +++ b/openbsc/src/gprs/gtphub.c @@ -1223,10 +1223,7 @@ static int gtphub_write(const struct osmo_fd *to, errno = 0; ssize_t sent = sendto(to->fd, buf, buf_len, 0, (struct sockaddr*)&to_addr->a, to_addr->l); - - if (to_addr) { - LOG(LOGL_DEBUG, "to %s\n", osmo_sockaddr_to_str(to_addr)); - } + LOG(LOGL_DEBUG, "to %s\n", osmo_sockaddr_to_str(to_addr)); if (sent == -1) { LOG(LOGL_ERROR, "error: %s\n", strerror(errno)); @@ -1712,6 +1709,7 @@ void gtphub_resolved_ggsn(struct gtphub *hub, const char *apn_oi_str, gtphub_port_ref_count_inc(pp); strncpy(ggsn->apn_oi_str, apn_oi_str, sizeof(ggsn->apn_oi_str)); + ggsn->apn_oi_str[sizeof(ggsn->apn_oi_str) - 1] = '\0'; ggsn->expiry_entry.del_cb = resolved_gssn_del_cb; expiry_add(&hub->expire_tei_maps, &ggsn->expiry_entry, now); @@ -1902,7 +1900,7 @@ int gtphub_start(struct gtphub *hub, struct gtphub_cfg *cfg) } for (plane_idx = 0; plane_idx < GTPH_PLANE_N; plane_idx++) { - if (hub->sgsn_proxy[plane_idx]) + if (hub->ggsn_proxy[plane_idx]) LOG(LOGL_NOTICE, "Using GGSN %s proxy %s\n", gtphub_plane_idx_names[plane_idx], gtphub_port_str(hub->ggsn_proxy[plane_idx])); -- 2.1.4
Since they (should) fix coverity complaints, I'm also merging these three to master myself. ~Neels On Tue, Nov 17, 2015 at 12:51:54PM +0100, Neels Hofmeyr wrote:
gtphub always wants to know the sender, hence make the from_addr pointer mandatory.
Fixes two coverity complaints (1339766, 1339764).
Sponsored-by: On-Waves ehi --- openbsc/src/gprs/gtphub.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/openbsc/src/gprs/gtphub.c b/openbsc/src/gprs/gtphub.c index e00d6cd..049e9f5 100644 --- a/openbsc/src/gprs/gtphub.c +++ b/openbsc/src/gprs/gtphub.c @@ -784,16 +784,16 @@ static int gtphub_bind_start(struct gtphub_bind *b, return 0; }
-/* Recv datagram from from->fd, optionally write sender's address to *from_addr. +/* Recv datagram from from->fd, write sender's address to *from_addr. * Return the number of bytes read, zero on error. */ static int gtphub_read(const struct osmo_fd *from, struct osmo_sockaddr *from_addr, uint8_t *buf, size_t buf_len) { - /* recvfrom requires the available length set in *from_addr_len. */ - if (from_addr) - from_addr->l = sizeof(from_addr->a); + OSMO_ASSERT(from_addr);
+ /* recvfrom requires the available length set in *from_addr_len. */ + from_addr->l = sizeof(from_addr->a); errno = 0; ssize_t received = recvfrom(from->fd, buf, buf_len, 0, (struct sockaddr*)&from_addr->a, @@ -807,12 +807,10 @@ static int gtphub_read(const struct osmo_fd *from, return 0; }
- if (from_addr) { - LOG(LOGL_DEBUG, "from %s\n", osmo_sockaddr_to_str(from_addr)); - } + LOG(LOGL_DEBUG, "Received %d bytes from %s\n%s\n", + (int)received, osmo_sockaddr_to_str(from_addr), + osmo_hexdump(buf, received));
- LOG(LOGL_DEBUG, "Received %d %s\n", - (int)received, osmo_hexdump(buf, received)); return received; }
-- 2.1.4
participants (1)
-
Neels Hofmeyr