Hi Ben, Question regarding patch: Shouldn't the fix be applied in flow extract code itself? I think the malformedness is evident during flow extraction. Might save you a few cycles/more secure.
On March 4, 2017 6:18:54 AM GMT+01:00, Ben Pfaff <[email protected]> wrote: >On Fri, Mar 03, 2017 at 05:00:38PM -0800, Daniele Di Proietto wrote: >> 2017-03-03 14:08 GMT-08:00 Ben Pfaff <[email protected]>: >> > Otherwise a malformed packet could cause a read up to about 40 >bytes past >> > the end of the packet. The packet would still likely be dropped >because >> > of checksum verification. >> > >> > Reported-by: Bhargava Shastry <[email protected]> >> > Signed-off-by: Ben Pfaff <[email protected]> >> >> Oops, thanks for the fix, Ben >> >> Fixes: a489b16854b5("conntrack: New userspace connection tracker.") >> >> One minor comment below, >> >> Acked-by: Daniele Di Proietto <[email protected]> >> >> >> > --- >> > lib/conntrack.c | 14 ++++++++++++-- >> > 1 file changed, 12 insertions(+), 2 deletions(-) >> > >> > diff --git a/lib/conntrack.c b/lib/conntrack.c >> > index 9bea3d93e4ad..9c1dd63648b8 100644 >> > --- a/lib/conntrack.c >> > +++ b/lib/conntrack.c >> > @@ -568,6 +568,10 @@ extract_l3_ipv6(struct conn_key *key, const >void *data, size_t size, >> > const char **new_data) >> > { >> > const struct ovs_16aligned_ip6_hdr *ip6 = data; >> > + if (size < sizeof *ip6) { >> > + return false; >> > + } >> > + >> >> We can read 'ip6->ip6_nxt' even though there's not enough data. It >> cannot happen >> for regular TCP and UDP packets (those are covered my >> miniflow_extract), but only >> when parsing the nested l3 header in an ICMP error message. >> >> The code has the same check two lines below, maybe we can reuse that. >> Technically >> the check is necessary only if new_data != NULL, as explained by the >comment >> above, but perhaps it's more clear to always perform it. > >Thanks for pointing out the duplicate check. I guess that I did not >read the code carefully enough. > >Usually, I would argue to always do the check, but I prefer to make >this >a minimal change. > >I will post a v2. -- Sent from my Android device with K-9 Mail. Please excuse my brevity. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
