fre. 17. jan. 2025, 14:56 skrev Eelco Chaudron <[email protected]>:
>
>
> On 13 Jan 2025, at 21:45, Frode Nordahl wrote:
>
> > Hello,
> >
> > This is a continuation of the work started in [0], which has gone
> > through a couple of iterations by Felix Huettner [1][2].
> >
> > As agreed in the OVN A/V Community meeting December 2nd 2024, I took
> > over herding these patches again.
> >
> > The main rationale for this is that I have done a fair bit of work on
> > Netlink related code before, so it would be a good way to spread the
> > workload of this epic, with the goal of getting the most out of this
> > community effort prior to looming freeze deadlines.
> >
> > Main change from previous iterations is:
> > * Series has been properly separated into logical units of change.
> > * Nexthops stored as a linked list as opposed to fixed size array.
> > * Support for parsing IPv4 routes over IPv6 next hops through the use
> > of the RTA_VIA attribute.
> > * As agreed the namespaces code proposed by Felix Huettner has been
> > dropped.
> > * More complete tests have been provided.
>
> Hi Frode,
>
> I received a message from Coverity indicating a potential memory leak. It
> seems this would only occur in cases where we encounter an invalid route
> message, but it might be a good idea to safeguard against it. Could you
> take a look and submit a follow-up patch?
>
Thanks for catching and reporting this, and sorry for the bug.
The logic for cleanup assumes rdnh will always be added to the list, but
then I apparently chose to skip that when processing the outer message with
a RTA_MULTIPATH attribute, presumably because its nexthop attributes will
be added when recursing, and the list element would be redundant.
Will work and post a fix at first opportunity.
--
Frode Nordahl
Here is the report:
>
> New defect(s) Reported-by: Coverity Scan
> Showing 1 of 1 defect(s)
>
>
> ** CID 485681: (RESOURCE_LEAK)
> /lib/route-table.c: 307 in route_table_parse__()
> /lib/route-table.c: 321 in route_table_parse__()
> /lib/route-table.c: 408 in route_table_parse__()
> /lib/route-table.c: 369 in route_table_parse__()
>
>
>
> ________________________________________________________________________________________________________
> *** CID 485681: (RESOURCE_LEAK)
> /lib/route-table.c: 307 in route_table_parse__()
> 301 change->rd.rtm_dst_len = rtm->rtm_dst_len;
> 302 change->rd.rtm_protocol = rtm->rtm_protocol;
> 303 change->rd.rtn_local = rtm->rtm_type == RTN_LOCAL;
> 304 if (attrs[RTA_OIF] && rtnh) {
> 305 VLOG_DBG_RL(&rl, "unexpected RTA_OIF attribute while
> parsing "
> 306 "nested RTA_MULTIPATH attributes");
> CID 485681: (RESOURCE_LEAK)
> Variable "rdnh" going out of scope leaks the storage it points to.
> 307 goto error_out;
> 308 }
> 309 if (attrs[RTA_OIF] || rtnh) {
> 310 rta_oif = rtnh ? rtnh->rtnh_ifindex
> 311 : nl_attr_get_u32(attrs[RTA_OIF]);
> 312
> /lib/route-table.c: 321 in route_table_parse__()
> 315
> 316 VLOG_DBG_RL(&rl, "could not find interface
> name[%u]: %s",
> 317 rta_oif, ovs_strerror(error));
> 318 if (error == ENXIO) {
> 319 change->relevant = false;
> 320 } else {
> CID 485681: (RESOURCE_LEAK)
> Variable "rdnh" going out of scope leaks the storage it points to.
> 321 goto error_out;
> 322 }
> 323 }
> 324 }
> 325
> 326 if (attrs[RTA_DST]) {
> /lib/route-table.c: 408 in route_table_parse__()
> 402 if (attrs[RTA_MULTIPATH]) {
> 403 const struct nlattr *nla;
> 404 size_t left;
> 405
> 406 if (rtnh) {
> 407 VLOG_DBG_RL(&rl, "unexpected nested RTA_MULTIPATH
> attribute");
> CID 485681: (RESOURCE_LEAK)
> Variable "rdnh" going out of scope leaks the storage it points to.
> 408 goto error_out;
> 409 }
> 410
> 411 NL_NESTED_FOR_EACH (nla, left, attrs[RTA_MULTIPATH]) {
> 412 struct route_table_msg mp_change;
> 413 struct rtnexthop *mp_rtnh;
> /lib/route-table.c: 369 in route_table_parse__()
> 363 const struct rtvia *rtvia =
> nl_attr_get(attrs[RTA_VIA]);
> 364 ovs_be32 addr;
> 365
> 366 if (attrs[RTA_GATEWAY]) {
> 367 VLOG_DBG_RL(&rl, "route message can not contain
> both "
> 368 "RTA_GATEWAY and RTA_VIA");
> CID 485681: (RESOURCE_LEAK)
> Variable "rdnh" going out of scope leaks the storage it points to.
> 369 goto error_out;
> 370 }
> 371
> 372 rdnh->family = rtvia->rtvia_family;
> 373
> 374 switch (rdnh->family) {
>
>
> Cheers,
>
> Eelco
>
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
