On Jul 15, Dumitru Ceara wrote:
> On 7/15/22 15:25, Lorenzo Bianconi wrote:
> > ofpbuf_put_zeros routine can rellocate the buffer if the requested size
> > is bigger than buffer tailroom. Reload ol pointer before running
> > ofpact_finish_LEARN in order to avoid a possible use-after-free in
> > add_lb_vip_hairpin_reply_action routine.
> >
> > Fixes: 022ea339c8 ("lflow: Use learn() action to generate LB hairpin reply
> > flows.")
> > Signed-off-by: Lorenzo Bianconi <[email protected]>
> > ---
>
> Hi Lorenzo,
>
> Nice catch!
>
> > controller/lflow.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/controller/lflow.c b/controller/lflow.c
> > index 934b23698..bba895815 100644
> > --- a/controller/lflow.c
> > +++ b/controller/lflow.c
> > @@ -1928,6 +1928,8 @@ add_lb_vip_hairpin_reply_action(struct in6_addr
> > *vip6, ovs_be32 vip,
> > src_imm = ofpbuf_put_zeros(ofpacts, OFPACT_ALIGN(imm_bytes));
> > memcpy(src_imm, &imm_reg_value, imm_bytes);
> >
> > + /* Reload ol pointer since ofpacts buffer can be reallocated. */
> > + ol = ofpacts->header;
>
> There's no guarantee that ofpacts doesn't already have another action
> before the learn action. It may change in the future. I think it's
> probably safer to:
>
> // Save offset before adding the learn action, in the
> // beginning of add_lb_vip_hairpin_reply_action().
> size_t ol_offset = ofpacts->size;
>
> [...]
> // Before finish_LEARN():
> ol = ofpbuf_at_assert(ofpacts, ol_offset, sizeof *ol);
>
> > ofpact_finish_LEARN(ofpacts, &ol);
> > }
> >
>
> Thanks,
> Dumitru
>
ack, I will fix it in v2.
Regards,
Lorenzo
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
