diff mbox series

[ovs-dev,ovn,1/2] DNSSL: copy dnssl string in order to avoid truncated value

Message ID f8c7eec5aadf658f66624ba6a84b2cebbca472ba.1578085180.git.lorenzo.bianconi@redhat.com
State Superseded
Headers show
Series fix DNSSL and Route Info Option in RA | expand

Commit Message

Lorenzo Bianconi Jan. 3, 2020, 9:01 p.m. UTC
ipv6_ra_send can run 2 times in a row before prepare_ipv6_ras updates
the dnss list. Clone the dnss dynamic string before running
packet_put_ra_dnssl_opt in order to avoid sending truncated dnssl list
on the wire.
Moreover move ip6_hdr definition just before accessing it because the
packet can be reallocated if the data area is not big enough for packet
content

Fixes: 5a12a940f ("Add DNSSL support to OVN")
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
---
 controller/pinctrl.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

Numan Siddique Jan. 7, 2020, 5:26 a.m. UTC | #1
On Sat, Jan 4, 2020 at 2:32 AM Lorenzo Bianconi
<lorenzo.bianconi@redhat.com> wrote:
>
> ipv6_ra_send can run 2 times in a row before prepare_ipv6_ras updates
> the dnss list. Clone the dnss dynamic string before running
> packet_put_ra_dnssl_opt in order to avoid sending truncated dnssl list
> on the wire.
> Moreover move ip6_hdr definition just before accessing it because the
> packet can be reallocated if the data area is not big enough for packet
> content
>
> Fixes: 5a12a940f ("Add DNSSL support to OVN")
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
> ---
>  controller/pinctrl.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index c886b21d9..bb1f0ab5a 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -2417,7 +2417,6 @@ packet_put_ra_dnssl_opt(struct dp_packet *b, ovs_be32 lifetime,
>  {
>      size_t prev_l4_size = dp_packet_l4_size(b);
>      size_t size = sizeof(struct ovs_nd_dnssl);
> -    struct ip6_hdr *nh = dp_packet_l3(b);
>      char *t0, *r0 = NULL, dnssl[255] = {};
>      int i = 0;
>
> @@ -2445,6 +2444,8 @@ packet_put_ra_dnssl_opt(struct dp_packet *b, ovs_be32 lifetime,
>          dnssl[i++] = 0;
>      }
>      size = ROUND_UP(size, 8);
> +
> +    struct ip6_hdr *nh = dp_packet_l3(b);
>      nh->ip6_plen = htons(prev_l4_size + size);
>
>      struct ovs_nd_dnssl *nd_dnssl = dp_packet_put_uninit(b, sizeof *nd_dnssl);
> @@ -2566,8 +2567,11 @@ ipv6_ra_send(struct rconn *swconn, struct ipv6_ra_state *ra)
>                                  &ra->config->rdnss);
>      }
>      if (ra->config->dnssl.length) {
> -        packet_put_ra_dnssl_opt(&packet, htonl(0xffffffff),
> -                                ra->config->dnssl.string);
> +        struct ds dnssl;
> +
> +        ds_clone(&dnssl, &ra->config->dnssl);
> +        packet_put_ra_dnssl_opt(&packet, htonl(0xffffffff), ds_cstr(&dnssl));
> +        ds_destroy(&dnssl);

Instead of ds_clone and ds_destroy, the function
packet_put_ra_dnssl_opt() can do xstrdup the string
and use that for strtok_r() operations. It can later free the string.
What do you think ?

Same comment for the patch 2 as well.

Thanks
Numan

>      }
>      if (ra->config->route_info.length) {
>          packet_put_ra_route_info_opt(&packet, htonl(0xffffffff),
> --
> 2.21.0
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
diff mbox series

Patch

diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index c886b21d9..bb1f0ab5a 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -2417,7 +2417,6 @@  packet_put_ra_dnssl_opt(struct dp_packet *b, ovs_be32 lifetime,
 {
     size_t prev_l4_size = dp_packet_l4_size(b);
     size_t size = sizeof(struct ovs_nd_dnssl);
-    struct ip6_hdr *nh = dp_packet_l3(b);
     char *t0, *r0 = NULL, dnssl[255] = {};
     int i = 0;
 
@@ -2445,6 +2444,8 @@  packet_put_ra_dnssl_opt(struct dp_packet *b, ovs_be32 lifetime,
         dnssl[i++] = 0;
     }
     size = ROUND_UP(size, 8);
+
+    struct ip6_hdr *nh = dp_packet_l3(b);
     nh->ip6_plen = htons(prev_l4_size + size);
 
     struct ovs_nd_dnssl *nd_dnssl = dp_packet_put_uninit(b, sizeof *nd_dnssl);
@@ -2566,8 +2567,11 @@  ipv6_ra_send(struct rconn *swconn, struct ipv6_ra_state *ra)
                                 &ra->config->rdnss);
     }
     if (ra->config->dnssl.length) {
-        packet_put_ra_dnssl_opt(&packet, htonl(0xffffffff),
-                                ra->config->dnssl.string);
+        struct ds dnssl;
+
+        ds_clone(&dnssl, &ra->config->dnssl);
+        packet_put_ra_dnssl_opt(&packet, htonl(0xffffffff), ds_cstr(&dnssl));
+        ds_destroy(&dnssl);
     }
     if (ra->config->route_info.length) {
         packet_put_ra_route_info_opt(&packet, htonl(0xffffffff),