neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-upf/+/30497 ) Change subject: GTP,UE addrs in osmo_sockaddr: assert( port == 0 ) ...................................................................... GTP,UE addrs in osmo_sockaddr: assert( port == 0 ) Assert that all port numbers in osmo_sockaddr parts of up_gtp_action are zero: uncover code paths that leak port numbers into the gtp_action API. GTP and UE addresses have no port information. Port numbers in GTP,UE addresses stored in struct osmo_sockaddr should be zero, so that - to-string conversion via osmo_sockaddr_to_str_c() returns only an IP address: for nftables rules and logging. - osmo_sockaddr_cmp() matches on identical IP addresses "only", without the port numbers causing mismatches: for finding tunnels and devs. Change-Id: If49f1e82e8cb92b7225e85a7c3b059e0f7f92fa3 --- M src/osmo-upf/upf_gtp.c M src/osmo-upf/upf_nft.c 2 files changed, 16 insertions(+), 0 deletions(-) Approvals: Jenkins Builder: Verified fixeria: Looks good to me, approved laforge: Looks good to me, but someone else must approve diff --git a/src/osmo-upf/upf_gtp.c b/src/osmo-upf/upf_gtp.c index f04dad1..adac836 100644 --- a/src/osmo-upf/upf_gtp.c +++ b/src/osmo-upf/upf_gtp.c @@ -323,10 +323,18 @@ return 0; } +#define tunend_desc_validate(TUN_DESC) \ + do { \ + OSMO_ASSERT(osmo_sockaddr_port(&(TUN_DESC)->access.gtp_local_addr.u.sa) == 0); \ + OSMO_ASSERT(osmo_sockaddr_port(&(TUN_DESC)->access.gtp_remote_addr.u.sa) == 0); \ + OSMO_ASSERT(osmo_sockaddr_port(&(TUN_DESC)->core.ue_local_addr.u.sa) == 0); \ + } while (0) + static struct upf_gtp_tunend *upf_gtp_tunend_alloc(struct upf_gtp_dev *dev, const struct upf_gtp_tunend_desc *desc) { struct upf_gtp_tunend *tun = talloc(dev, struct upf_gtp_tunend); OSMO_ASSERT(tun); + tunend_desc_validate(desc); *tun = (struct upf_gtp_tunend){ .dev = dev, .desc = *desc, @@ -386,6 +394,7 @@ static struct upf_gtp_tunend *upf_gtp_dev_tunend_find(struct upf_gtp_dev *dev, const struct upf_gtp_tunend_desc *tun_desc) { struct upf_gtp_tunend *tun; + tunend_desc_validate(tun_desc); llist_for_each_entry(tun, &dev->tunnels, entry) { if (upf_gtp_tunend_desc_cmp(tun_desc, &tun->desc)) continue; @@ -397,6 +406,7 @@ int upf_gtp_dev_tunend_add(struct upf_gtp_dev *dev, const struct upf_gtp_tunend_desc *tun_desc) { struct upf_gtp_tunend *tun; + tunend_desc_validate(tun_desc); tun = upf_gtp_dev_tunend_find(dev, tun_desc); if (!tun) tun = upf_gtp_tunend_alloc(dev, tun_desc); @@ -409,6 +419,7 @@ { struct upf_gtp_tunend *tun; int rc; + tunend_desc_validate(tun_desc); tun = upf_gtp_dev_tunend_find(dev, tun_desc); if (!tun) return 0; diff --git a/src/osmo-upf/upf_nft.c b/src/osmo-upf/upf_nft.c index c34cbfb..8f59d01 100644 --- a/src/osmo-upf/upf_nft.c +++ b/src/osmo-upf/upf_nft.c @@ -191,6 +191,11 @@ static void upf_nft_args_from_tunmap_desc(struct upf_nft_args *args, const struct upf_nft_tunmap_desc *tunmap) { + OSMO_ASSERT(osmo_sockaddr_port(&tunmap->access.gtp_remote_addr.u.sa) == 0); + OSMO_ASSERT(osmo_sockaddr_port(&tunmap->access.gtp_local_addr.u.sa) == 0); + OSMO_ASSERT(osmo_sockaddr_port(&tunmap->core.gtp_remote_addr.u.sa) == 0); + OSMO_ASSERT(osmo_sockaddr_port(&tunmap->core.gtp_local_addr.u.sa) == 0); + *args = (struct upf_nft_args){ .table_name = g_upf->nft.table_name, .chain_id = tunmap->id, -- To view, visit https://gerrit.osmocom.org/c/osmo-upf/+/30497 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-upf Gerrit-Branch: master Gerrit-Change-Id: If49f1e82e8cb92b7225e85a7c3b059e0f7f92fa3 Gerrit-Change-Number: 30497 Gerrit-PatchSet: 3 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: merged