[ovs-dev] [PATCH ovn] pinctrl: Fix icmp6 packet corruption issue

[Dumitru Ceara]

On 5/12/20 11:25 AM, numans at ovn.org wrote:
> From: Numan Siddique <numans at ovn.org>
> 
> The commit f792b1a00b43("Fix ACL reject action for UDP packets.")
> didn't updated the 'struct ip6_hdr' pointer after calling
> dp_packet_put(), as dp_packet_put() can reallocate memory making the
> old references to packet pointers invalid.
> 
> This patch fixes this issue.
> 
> Fixes: f792b1a00b43("Fix ACL reject action for UDP packets.")
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1834655
> Signed-off-by: Numan Siddique <numans at ovn.org>

Hi Numan,

I think I'd change the
foofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoo
to something a bit different but except for that:

Acked-by: Dumitru Ceara <dceara at redhat.com>

Thanks,
Dumitru

> ---
>  controller/pinctrl.c | 4 ++--
>  tests/system-ovn.at  | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index 6b0ac3483..d976ec82b 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -1570,8 +1570,6 @@ pinctrl_handle_icmp(struct rconn *swconn, const struct flow *ip_flow,
>          }
>          ih->icmp6_base.icmp6_cksum = 0;
>  
> -        nh = dp_packet_l3(&packet);
> -
>          /* RFC 4443: 3.1.
>           *
>           * 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> @@ -1594,9 +1592,11 @@ pinctrl_handle_icmp(struct rconn *swconn, const struct flow *ip_flow,
>          }
>  
>          dp_packet_put(&packet, in_ip, in_ip_len);
> +        nh = dp_packet_l3(&packet);
>          nh->ip6_plen = htons(ICMP6_DATA_HEADER_LEN + in_ip_len);
>  
>          icmpv6_csum = packet_csum_pseudoheader6(dp_packet_l3(&packet));
> +        ih = dp_packet_l4(&packet);
>          ih->icmp6_base.icmp6_cksum = csum_finish(
>              csum_continue(icmpv6_csum, ih,
>                            in_ip_len + ICMP6_DATA_HEADER_LEN));
> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
> index 9a5ef1ec3..d0240840d 100644
> --- a/tests/system-ovn.at
> +++ b/tests/system-ovn.at
> @@ -3967,7 +3967,7 @@ OVS_WAIT_UNTIL([
>  NS_CHECK_EXEC([sw0-p1-rej], [tcpdump -n -c 1 -i sw0-p1-rej udp port 90 > sw0-p1-rej-udp.pcap &], [0])
>  NS_CHECK_EXEC([sw0-p1-rej], [tcpdump -n -c 1 -i sw0-p1-rej icmp > sw0-p1-rej-icmp.pcap &], [0])
>  
> -echo "foo" > foo
> +echo "foofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoo" > foo
>  OVS_WAIT_UNTIL([
>      ip netns exec sw0-p1-rej nc -u 10.0.0.4 90 < foo
>      c=$(cat sw0-p1-rej-icmp.pcap | grep \
>