Thx, applied to master,
On Fri, Aug 15, 2014 at 1:33 PM, Ethan Jackson <et...@nicira.com> wrote:
> Acked-by: Ethan Jackson <et...@nicira.com>
>
> Thanks for taking care of this.
>
> Ethan
>
> On Fri, Aug 15, 2014 at 1:15 AM, Alex Wang <al...@nicira.com> wrote:
> > Commit cc377352d (ofproto: Reorganize in preparation for direct
> > dpdk upcalls.) introduced the bug that uses variable defined on
> > the stack inside while loop for reading dpif upcalls and keeps
> > reference to attributes of the variable within the same function
> > after the stack is cleared. This bug can cause ovs abort.
> >
> > This commit fixes the above issue by defining an array of the
> > variable on the function stack.
> >
> > Signed-off-by: Alex Wang <al...@nicira.com>
> > ---
> > ofproto/ofproto-dpif-upcall.c | 27 ++++++++++++++-------------
> > 1 file changed, 14 insertions(+), 13 deletions(-)
> >
> > diff --git a/ofproto/ofproto-dpif-upcall.c
> b/ofproto/ofproto-dpif-upcall.c
> > index 180684c..9f68a7d 100644
> > --- a/ofproto/ofproto-dpif-upcall.c
> > +++ b/ofproto/ofproto-dpif-upcall.c
> > @@ -574,55 +574,56 @@ recv_upcalls(struct handler *handler)
> > struct udpif *udpif = handler->udpif;
> > uint64_t recv_stubs[UPCALL_MAX_BATCH][512 / 8];
> > struct ofpbuf recv_bufs[UPCALL_MAX_BATCH];
> > + struct dpif_upcall dupcalls[UPCALL_MAX_BATCH];
> > struct upcall upcalls[UPCALL_MAX_BATCH];
> > size_t n_upcalls, i;
> >
> > n_upcalls = 0;
> > while (n_upcalls < UPCALL_MAX_BATCH) {
> > struct ofpbuf *recv_buf = &recv_bufs[n_upcalls];
> > + struct dpif_upcall *dupcall = &dupcalls[n_upcalls];
> > struct upcall *upcall = &upcalls[n_upcalls];
> > - struct dpif_upcall dupcall;
> > struct pkt_metadata md;
> > struct flow flow;
> > int error;
> >
> > ofpbuf_use_stub(recv_buf, recv_stubs[n_upcalls],
> > sizeof recv_stubs[n_upcalls]);
> > - if (dpif_recv(udpif->dpif, handler->handler_id, &dupcall,
> recv_buf)) {
> > + if (dpif_recv(udpif->dpif, handler->handler_id, dupcall,
> recv_buf)) {
> > ofpbuf_uninit(recv_buf);
> > break;
> > }
> >
> > - if (odp_flow_key_to_flow(dupcall.key, dupcall.key_len, &flow)
> > + if (odp_flow_key_to_flow(dupcall->key, dupcall->key_len, &flow)
> > == ODP_FIT_ERROR) {
> > goto free_dupcall;
> > }
> >
> > - error = upcall_receive(upcall, udpif->backer, &dupcall.packet,
> > - dupcall.type, dupcall.userdata, &flow);
> > + error = upcall_receive(upcall, udpif->backer, &dupcall->packet,
> > + dupcall->type, dupcall->userdata, &flow);
> > if (error) {
> > if (error == ENODEV) {
> > /* Received packet on datapath port for which we
> couldn't
> > * associate an ofproto. This can happen if a port is
> removed
> > * while traffic is being received. Print a
> rate-limited
> > * message in case it happens frequently. */
> > - dpif_flow_put(udpif->dpif, DPIF_FP_CREATE, dupcall.key,
> > - dupcall.key_len, NULL, 0, NULL, 0, NULL);
> > + dpif_flow_put(udpif->dpif, DPIF_FP_CREATE, dupcall->key,
> > + dupcall->key_len, NULL, 0, NULL, 0, NULL);
> > VLOG_INFO_RL(&rl, "received packet on unassociated
> datapath "
> > "port %"PRIu32, flow.in_port.odp_port);
> > }
> > goto free_dupcall;
> > }
> >
> > - upcall->key = dupcall.key;
> > - upcall->key_len = dupcall.key_len;
> > + upcall->key = dupcall->key;
> > + upcall->key_len = dupcall->key_len;
> >
> > - if (vsp_adjust_flow(upcall->ofproto, &flow, &dupcall.packet)) {
> > + if (vsp_adjust_flow(upcall->ofproto, &flow, &dupcall->packet)) {
> > upcall->vsp_adjusted = true;
> > }
> >
> > md = pkt_metadata_from_flow(&flow);
> > - flow_extract(&dupcall.packet, &md, &flow);
> > + flow_extract(&dupcall->packet, &md, &flow);
> >
> > error = process_upcall(udpif, upcall, NULL);
> > if (error) {
> > @@ -635,14 +636,14 @@ recv_upcalls(struct handler *handler)
> > cleanup:
> > upcall_uninit(upcall);
> > free_dupcall:
> > - ofpbuf_uninit(&dupcall.packet);
> > + ofpbuf_uninit(&dupcall->packet);
> > ofpbuf_uninit(recv_buf);
> > }
> >
> > if (n_upcalls) {
> > handle_upcalls(handler->udpif, upcalls, n_upcalls);
> > for (i = 0; i < n_upcalls; i++) {
> > - ofpbuf_uninit(CONST_CAST(struct ofpbuf *,
> upcalls[i].packet));
> > + ofpbuf_uninit(&dupcalls[i].packet);
> > ofpbuf_uninit(&recv_bufs[i]);
> > upcall_uninit(&upcalls[i]);
> > }
> > --
> > 1.7.9.5
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
