On 9 Sep 2024, at 6:55, Mike Pattrick wrote:
> Currently the error conditions in vconn_dump_flows() don't handle
> freeing memory in a consistent fashion. This can make it possible to
> reference memory after it's freed.
>
> This patch attempts to handle errors consistently. Error conditions will
> always cause memory to be freed and then that memory will never be
> referenced.
>
> Fixes: d444a914fdbd ("ovn-trace: New --ovs option to also print OpenFlow
> flows.")
> Signed-off-by: Mike Pattrick <[email protected]>
The changes look good to me.
Acked-by: Eelco Chaudron <[email protected]>
//Eelco
Had to spend some time figuring out why ‘*replyp = NULL;’ had to be moved out
of the !more part…
> ---
> v3: Simplified logic per review.
> ---
> lib/vconn.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/lib/vconn.c b/lib/vconn.c
> index e9603432d..4b1c262ea 100644
> --- a/lib/vconn.c
> +++ b/lib/vconn.c
> @@ -1017,6 +1017,8 @@ recv_flow_stats_reply(struct vconn *vconn, ovs_be32
> send_xid,
> VLOG_WARN_RL(&rl, "received bad reply: %s",
> ofp_to_string(reply->data, reply->size,
> NULL, NULL, 1));
> + ofpbuf_delete(reply);
> + *replyp = NULL;
> return EPROTO;
> }
> }
> @@ -1031,9 +1033,9 @@ recv_flow_stats_reply(struct vconn *vconn, ovs_be32
> send_xid,
> case EOF:
> more = ofpmp_more(reply->header);
> ofpbuf_delete(reply);
> + *replyp = NULL;
> reply = NULL;
> if (!more) {
> - *replyp = NULL;
> return EOF;
> }
> break;
> @@ -1041,6 +1043,8 @@ recv_flow_stats_reply(struct vconn *vconn, ovs_be32
> send_xid,
> default:
> VLOG_WARN_RL(&rl, "parse error in reply (%s)",
> ofperr_to_string(retval));
> + ofpbuf_delete(reply);
> + *replyp = NULL;
> return EPROTO;
> }
> }
> --
> 2.43.5
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
