Re: [PATCH net 1/2] openvswitch: fix stack OOB read while fragmenting IPv4 packets

["Eelco Chaudron" <echaudro@redhat.com>]

On 19 Apr 2021, at 17:23, Davide Caratti wrote:

> running openvswitch on kernels built with KASAN, it's possible to see 
> the
> following splat while testing fragmentation of IPv4 packets:

<SNIP>

> for IPv4 packets, ovs_fragment() uses a temporary struct dst_entry. 
> Then,
> in the following call graph:
>
>   ip_do_fragment()
>     ip_skb_dst_mtu()
>       ip_dst_mtu_maybe_forward()
>         ip_mtu_locked()
>
> the pointer to struct dst_entry is used as pointer to struct rtable: 
> this
> turns the access to struct members like rt_mtu_locked into an OOB read 
> in
> the stack. Fix this changing the temporary variable used for IPv4 
> packets
> in ovs_fragment(), similarly to what is done for IPv6 few lines below.
>
> Fixes: d52e5a7e7ca4 ("ipv4: lock mtu in fnhe when received PMTU < 
> net.ipv4.route.min_pmt")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>

The fix looks good to me, however isn’t the real root cause 
ip_mtu_locked() who casts struct dst_entry to struct rtable (not even 
using container_of())?

I do not know details in this area of the code, so maybe it’s just 
fine to always assume dst_entry is part of a rtable struct, as I see 
other core functions do the same 
ipv4_neigh_lookup()/ipv4_confirm_neigh().


Acked-by: Eelco Chaudron <echaudro@redhat.com>