From: Saladin EL Ouali <[email protected]> This adds a bounds check in tun_metadata_from_geneve_nlattr() to ensure that attr_len does not exceed the destination buffer size (256 bytes) before performing memcpy().
While the kernel module validates Geneve option lengths before passing data to userspace via netlink, adding redundant validation in userspace provides defense-in-depth. If the kernel were to pass a malformed netlink message (due to a bug or compromise), userspace would previously overflow the tun->metadata.opts.gnv buffer. This was discussed with Ilya Maximets, who noted: "In general, we trust the kernel to give us correct data. If we can't trust the kernel, there is not much we can do. It makes sense to add a check in userspace parsing, but it's not a security concern." The function now returns -EINVAL on overflow and 0 on success, with callers in odp-util.c handling the error appropriately. Signed-off-by: Saladin EL Ouali <[email protected]> --- lib/odp-util.c | 6 +++++- lib/tun-metadata.c | 7 ++++++- lib/tun-metadata.h | 2 +- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/lib/odp-util.c b/lib/odp-util.c index ee1868202..d87506193 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -3255,7 +3255,11 @@ odp_tun_key_from_attr__(const struct nlattr *attr, bool is_mask, break; } case OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS: - tun_metadata_from_geneve_nlattr(a, is_mask, tun); + if (tun_metadata_from_geneve_nlattr(a, is_mask, tun)) { + odp_parse_error(&rl, errorp, + "Geneve options length exceeds maximum"); + return ODP_FIT_ERROR; + } break; case OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS: { const struct erspan_metadata *opts = nl_attr_get(a); diff --git a/lib/tun-metadata.c b/lib/tun-metadata.c index af0bcbde8..1561da35f 100644 --- a/lib/tun-metadata.c +++ b/lib/tun-metadata.c @@ -605,12 +605,15 @@ tun_metadata_del_entry(struct tun_table *map, uint8_t idx) * on the wire. By always using UDPIF format, this allows us to process the * flow key without any knowledge of the mapping table. We can do the * conversion later if necessary. */ -void +int tun_metadata_from_geneve_nlattr(const struct nlattr *attr, bool is_mask, struct flow_tnl *tun) { int attr_len = nl_attr_get_size(attr); + if (attr_len > sizeof(tun->metadata.opts.gnv)) { + return -EINVAL; + } memcpy(tun->metadata.opts.gnv, nl_attr_get(attr), attr_len); tun->flags |= FLOW_TNL_F_UDPIF; @@ -622,6 +625,8 @@ tun_metadata_from_geneve_nlattr(const struct nlattr *attr, bool is_mask, * at the beginning but with additional options after. */ tun->metadata.present.len = 0xff; } + return 0; } /* Converts from the flat Geneve options representation extracted directly diff --git a/lib/tun-metadata.h b/lib/tun-metadata.h index 67dedae25..affc8c268 100644 --- a/lib/tun-metadata.h +++ b/lib/tun-metadata.h @@ -54,7 +54,7 @@ void tun_metadata_set_match(const struct mf_field *, char **err_str); void tun_metadata_get_fmd(const struct flow_tnl *, struct match *flow_metadata); -void tun_metadata_from_geneve_nlattr(const struct nlattr *attr, bool is_mask, +int tun_metadata_from_geneve_nlattr(const struct nlattr *attr, bool is_mask, struct flow_tnl *tun); void tun_metadata_to_geneve_nlattr(const struct flow_tnl *tun, const struct flow_tnl *flow, -- 2.49.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
