| Message ID | 20220725151906.677460-1-amusil@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,v2] controller: Fix misaligned access to ip6_hdr | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
I forgot to add the v2 comment, so for clarification just the commit title has changed between the versions. Thanks, Ales On Mon, Jul 25, 2022 at 5:19 PM Ales Musil <amusil@redhat.com> wrote: > The ip6_hdr is aligned to 4 bytes, but the pointer > from dp_packet_l3 is aligned to 2 bytes. Use > ovs_16aligned_ip6_hdr instead to get 2 bytes alignment. > > Signed-off-by: Ales Musil <amusil@redhat.com> > --- > controller/pinctrl.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > index 38e8590af..21c78bed4 100644 > --- a/controller/pinctrl.c > +++ b/controller/pinctrl.c > @@ -887,7 +887,9 @@ pinctrl_parse_dhcpv6_reply(struct dp_packet *pkt_in, > OVS_REQUIRES(pinctrl_mutex) > { > struct eth_header *eth = dp_packet_eth(pkt_in); > - struct ip6_hdr *in_ip = dp_packet_l3(pkt_in); > + struct ovs_16aligned_ip6_hdr *in_ip = dp_packet_l3(pkt_in); > + struct in6_addr ip6_src; > + memcpy(&ip6_src, &in_ip->ip6_src, sizeof ip6_src); > struct udp_header *udp_in = dp_packet_l4(pkt_in); > unsigned char *in_dhcpv6_data = (unsigned char *)(udp_in + 1); > size_t dlen = MIN(ntohs(udp_in->udp_len), dp_packet_l4_size(pkt_in)); > @@ -971,7 +973,7 @@ pinctrl_parse_dhcpv6_reply(struct dp_packet *pkt_in, > " aid %d", ip6_s, prefix, prefix_len, aid); > } > pinctrl_prefixd_state_handler(ip_flow, ipv6, aid, eth->eth_src, > - in_ip->ip6_src, prefix_len, t1, t2, > + ip6_src, prefix_len, t1, t2, > plife_time, vlife_time, uuid, > uuid_len); > } else if (uuid) { > free(uuid); > @@ -3736,7 +3738,7 @@ packet_put_ra_rdnss_opt(struct dp_packet *b, uint8_t > num, > ovs_be32 lifetime, const struct in6_addr *dns) > { > size_t prev_l4_size = dp_packet_l4_size(b); > - struct ip6_hdr *nh = dp_packet_l3(b); > + struct ovs_16aligned_ip6_hdr *nh = dp_packet_l3(b); > size_t len = 2 * num + 1; > > nh->ip6_plen = htons(prev_l4_size + len * 8); > @@ -3771,7 +3773,7 @@ packet_put_ra_dnssl_opt(struct dp_packet *b, > ovs_be32 lifetime, > return; > } > > - struct ip6_hdr *nh = dp_packet_l3(b); > + struct ovs_16aligned_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); > @@ -3850,7 +3852,7 @@ packet_put_ra_route_info_opt(struct dp_packet *b, > ovs_be32 lifetime, > } > } > > - struct ip6_hdr *nh = dp_packet_l3(b); > + struct ovs_16aligned_ip6_hdr *nh = dp_packet_l3(b); > nh->ip6_plen = htons(prev_l4_size + size); > struct ovs_ra_msg *ra = dp_packet_l4(b); > ra->icmph.icmp6_cksum = 0; > -- > 2.35.3 > >
Thanks for the clarification Ales. Acked-by: Mark Michelson <mmichels@redhat.com> On 7/26/22 01:23, Ales Musil wrote: > I forgot to add the v2 comment, so for clarification just the commit title > has changed between the versions. > > Thanks, > Ales > > On Mon, Jul 25, 2022 at 5:19 PM Ales Musil <amusil@redhat.com> wrote: > >> The ip6_hdr is aligned to 4 bytes, but the pointer >> from dp_packet_l3 is aligned to 2 bytes. Use >> ovs_16aligned_ip6_hdr instead to get 2 bytes alignment. >> >> Signed-off-by: Ales Musil <amusil@redhat.com> >> --- >> controller/pinctrl.c | 12 +++++++----- >> 1 file changed, 7 insertions(+), 5 deletions(-) >> >> diff --git a/controller/pinctrl.c b/controller/pinctrl.c >> index 38e8590af..21c78bed4 100644 >> --- a/controller/pinctrl.c >> +++ b/controller/pinctrl.c >> @@ -887,7 +887,9 @@ pinctrl_parse_dhcpv6_reply(struct dp_packet *pkt_in, >> OVS_REQUIRES(pinctrl_mutex) >> { >> struct eth_header *eth = dp_packet_eth(pkt_in); >> - struct ip6_hdr *in_ip = dp_packet_l3(pkt_in); >> + struct ovs_16aligned_ip6_hdr *in_ip = dp_packet_l3(pkt_in); >> + struct in6_addr ip6_src; >> + memcpy(&ip6_src, &in_ip->ip6_src, sizeof ip6_src); >> struct udp_header *udp_in = dp_packet_l4(pkt_in); >> unsigned char *in_dhcpv6_data = (unsigned char *)(udp_in + 1); >> size_t dlen = MIN(ntohs(udp_in->udp_len), dp_packet_l4_size(pkt_in)); >> @@ -971,7 +973,7 @@ pinctrl_parse_dhcpv6_reply(struct dp_packet *pkt_in, >> " aid %d", ip6_s, prefix, prefix_len, aid); >> } >> pinctrl_prefixd_state_handler(ip_flow, ipv6, aid, eth->eth_src, >> - in_ip->ip6_src, prefix_len, t1, t2, >> + ip6_src, prefix_len, t1, t2, >> plife_time, vlife_time, uuid, >> uuid_len); >> } else if (uuid) { >> free(uuid); >> @@ -3736,7 +3738,7 @@ packet_put_ra_rdnss_opt(struct dp_packet *b, uint8_t >> num, >> ovs_be32 lifetime, const struct in6_addr *dns) >> { >> size_t prev_l4_size = dp_packet_l4_size(b); >> - struct ip6_hdr *nh = dp_packet_l3(b); >> + struct ovs_16aligned_ip6_hdr *nh = dp_packet_l3(b); >> size_t len = 2 * num + 1; >> >> nh->ip6_plen = htons(prev_l4_size + len * 8); >> @@ -3771,7 +3773,7 @@ packet_put_ra_dnssl_opt(struct dp_packet *b, >> ovs_be32 lifetime, >> return; >> } >> >> - struct ip6_hdr *nh = dp_packet_l3(b); >> + struct ovs_16aligned_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); >> @@ -3850,7 +3852,7 @@ packet_put_ra_route_info_opt(struct dp_packet *b, >> ovs_be32 lifetime, >> } >> } >> >> - struct ip6_hdr *nh = dp_packet_l3(b); >> + struct ovs_16aligned_ip6_hdr *nh = dp_packet_l3(b); >> nh->ip6_plen = htons(prev_l4_size + size); >> struct ovs_ra_msg *ra = dp_packet_l4(b); >> ra->icmph.icmp6_cksum = 0; >> -- >> 2.35.3 >> >> >
On Tue, Aug 9, 2022 at 5:49 AM Mark Michelson <mmichels@redhat.com> wrote: > > Thanks for the clarification Ales. > > Acked-by: Mark Michelson <mmichels@redhat.com> Thanks. I applied this patch to the main branch. Numan > > On 7/26/22 01:23, Ales Musil wrote: > > I forgot to add the v2 comment, so for clarification just the commit title > > has changed between the versions. > > > > Thanks, > > Ales > > > > On Mon, Jul 25, 2022 at 5:19 PM Ales Musil <amusil@redhat.com> wrote: > > > >> The ip6_hdr is aligned to 4 bytes, but the pointer > >> from dp_packet_l3 is aligned to 2 bytes. Use > >> ovs_16aligned_ip6_hdr instead to get 2 bytes alignment. > >> > >> Signed-off-by: Ales Musil <amusil@redhat.com> > >> --- > >> controller/pinctrl.c | 12 +++++++----- > >> 1 file changed, 7 insertions(+), 5 deletions(-) > >> > >> diff --git a/controller/pinctrl.c b/controller/pinctrl.c > >> index 38e8590af..21c78bed4 100644 > >> --- a/controller/pinctrl.c > >> +++ b/controller/pinctrl.c > >> @@ -887,7 +887,9 @@ pinctrl_parse_dhcpv6_reply(struct dp_packet *pkt_in, > >> OVS_REQUIRES(pinctrl_mutex) > >> { > >> struct eth_header *eth = dp_packet_eth(pkt_in); > >> - struct ip6_hdr *in_ip = dp_packet_l3(pkt_in); > >> + struct ovs_16aligned_ip6_hdr *in_ip = dp_packet_l3(pkt_in); > >> + struct in6_addr ip6_src; > >> + memcpy(&ip6_src, &in_ip->ip6_src, sizeof ip6_src); > >> struct udp_header *udp_in = dp_packet_l4(pkt_in); > >> unsigned char *in_dhcpv6_data = (unsigned char *)(udp_in + 1); > >> size_t dlen = MIN(ntohs(udp_in->udp_len), dp_packet_l4_size(pkt_in)); > >> @@ -971,7 +973,7 @@ pinctrl_parse_dhcpv6_reply(struct dp_packet *pkt_in, > >> " aid %d", ip6_s, prefix, prefix_len, aid); > >> } > >> pinctrl_prefixd_state_handler(ip_flow, ipv6, aid, eth->eth_src, > >> - in_ip->ip6_src, prefix_len, t1, t2, > >> + ip6_src, prefix_len, t1, t2, > >> plife_time, vlife_time, uuid, > >> uuid_len); > >> } else if (uuid) { > >> free(uuid); > >> @@ -3736,7 +3738,7 @@ packet_put_ra_rdnss_opt(struct dp_packet *b, uint8_t > >> num, > >> ovs_be32 lifetime, const struct in6_addr *dns) > >> { > >> size_t prev_l4_size = dp_packet_l4_size(b); > >> - struct ip6_hdr *nh = dp_packet_l3(b); > >> + struct ovs_16aligned_ip6_hdr *nh = dp_packet_l3(b); > >> size_t len = 2 * num + 1; > >> > >> nh->ip6_plen = htons(prev_l4_size + len * 8); > >> @@ -3771,7 +3773,7 @@ packet_put_ra_dnssl_opt(struct dp_packet *b, > >> ovs_be32 lifetime, > >> return; > >> } > >> > >> - struct ip6_hdr *nh = dp_packet_l3(b); > >> + struct ovs_16aligned_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); > >> @@ -3850,7 +3852,7 @@ packet_put_ra_route_info_opt(struct dp_packet *b, > >> ovs_be32 lifetime, > >> } > >> } > >> > >> - struct ip6_hdr *nh = dp_packet_l3(b); > >> + struct ovs_16aligned_ip6_hdr *nh = dp_packet_l3(b); > >> nh->ip6_plen = htons(prev_l4_size + size); > >> struct ovs_ra_msg *ra = dp_packet_l4(b); > >> ra->icmph.icmp6_cksum = 0; > >> -- > >> 2.35.3 > >> > >> > > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/controller/pinctrl.c b/controller/pinctrl.c index 38e8590af..21c78bed4 100644 --- a/controller/pinctrl.c +++ b/controller/pinctrl.c @@ -887,7 +887,9 @@ pinctrl_parse_dhcpv6_reply(struct dp_packet *pkt_in, OVS_REQUIRES(pinctrl_mutex) { struct eth_header *eth = dp_packet_eth(pkt_in); - struct ip6_hdr *in_ip = dp_packet_l3(pkt_in); + struct ovs_16aligned_ip6_hdr *in_ip = dp_packet_l3(pkt_in); + struct in6_addr ip6_src; + memcpy(&ip6_src, &in_ip->ip6_src, sizeof ip6_src); struct udp_header *udp_in = dp_packet_l4(pkt_in); unsigned char *in_dhcpv6_data = (unsigned char *)(udp_in + 1); size_t dlen = MIN(ntohs(udp_in->udp_len), dp_packet_l4_size(pkt_in)); @@ -971,7 +973,7 @@ pinctrl_parse_dhcpv6_reply(struct dp_packet *pkt_in, " aid %d", ip6_s, prefix, prefix_len, aid); } pinctrl_prefixd_state_handler(ip_flow, ipv6, aid, eth->eth_src, - in_ip->ip6_src, prefix_len, t1, t2, + ip6_src, prefix_len, t1, t2, plife_time, vlife_time, uuid, uuid_len); } else if (uuid) { free(uuid); @@ -3736,7 +3738,7 @@ packet_put_ra_rdnss_opt(struct dp_packet *b, uint8_t num, ovs_be32 lifetime, const struct in6_addr *dns) { size_t prev_l4_size = dp_packet_l4_size(b); - struct ip6_hdr *nh = dp_packet_l3(b); + struct ovs_16aligned_ip6_hdr *nh = dp_packet_l3(b); size_t len = 2 * num + 1; nh->ip6_plen = htons(prev_l4_size + len * 8); @@ -3771,7 +3773,7 @@ packet_put_ra_dnssl_opt(struct dp_packet *b, ovs_be32 lifetime, return; } - struct ip6_hdr *nh = dp_packet_l3(b); + struct ovs_16aligned_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); @@ -3850,7 +3852,7 @@ packet_put_ra_route_info_opt(struct dp_packet *b, ovs_be32 lifetime, } } - struct ip6_hdr *nh = dp_packet_l3(b); + struct ovs_16aligned_ip6_hdr *nh = dp_packet_l3(b); nh->ip6_plen = htons(prev_l4_size + size); struct ovs_ra_msg *ra = dp_packet_l4(b); ra->icmph.icmp6_cksum = 0;
The ip6_hdr is aligned to 4 bytes, but the pointer from dp_packet_l3 is aligned to 2 bytes. Use ovs_16aligned_ip6_hdr instead to get 2 bytes alignment. Signed-off-by: Ales Musil <amusil@redhat.com> --- controller/pinctrl.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)