| Message ID | 1531239309-18700-1-git-send-email-dlu998@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,v1] conntrack: Fix conn_update_state_alg use after free. | expand |
On Tue, Jul 10, 2018 at 09:15:09AM -0700, Darrell Ball wrote: > Fixes: bd5e81a0e596 ("Userspace Datapath: Add ALG infra and FTP.") > Signed-off-by: Darrell Ball <dlu998@gmail.com> This could use a little longer commit message. Is the following accurate: When conn_update_state() returns true, it has freed 'conn' which therefore should not be passed into handle_ftp_ctl(). Thanks, Ben. > --- > > Needs backporting as far back as 2.8. > > lib/conntrack.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/lib/conntrack.c b/lib/conntrack.c > index e1c1f2e..b818584 100644 > --- a/lib/conntrack.c > +++ b/lib/conntrack.c > @@ -1159,8 +1159,11 @@ conn_update_state_alg(struct conntrack *ct, struct dp_packet *pkt, > } else { > *create_new_conn = conn_update_state(ct, pkt, ctx, &conn, now, > bucket); > - handle_ftp_ctl(ct, ctx, pkt, conn, now, CT_FTP_CTL_OTHER, > - !!nat_action_info); > + > + if (*create_new_conn == false) { > + handle_ftp_ctl(ct, ctx, pkt, conn, now, CT_FTP_CTL_OTHER, > + !!nat_action_info); > + } > } > return true; > } > -- > 1.9.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
sorry, I only had a few minutes in the morning I sent a V2, with a proper commit message. https://patchwork.ozlabs.org/patch/942279/ Thanks Darrell On Tue, Jul 10, 2018 at 1:27 PM, Ben Pfaff <blp@ovn.org> wrote: > On Tue, Jul 10, 2018 at 09:15:09AM -0700, Darrell Ball wrote: > > Fixes: bd5e81a0e596 ("Userspace Datapath: Add ALG infra and FTP.") > > Signed-off-by: Darrell Ball <dlu998@gmail.com> > > This could use a little longer commit message. Is the following > accurate: > > When conn_update_state() returns true, it has freed 'conn' which > therefore should not be passed into handle_ftp_ctl(). > > Thanks, > > Ben. > > > --- > > > > Needs backporting as far back as 2.8. > > > > lib/conntrack.c | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/lib/conntrack.c b/lib/conntrack.c > > index e1c1f2e..b818584 100644 > > --- a/lib/conntrack.c > > +++ b/lib/conntrack.c > > @@ -1159,8 +1159,11 @@ conn_update_state_alg(struct conntrack *ct, > struct dp_packet *pkt, > > } else { > > *create_new_conn = conn_update_state(ct, pkt, ctx, &conn, > now, > > bucket); > > - handle_ftp_ctl(ct, ctx, pkt, conn, now, CT_FTP_CTL_OTHER, > > - !!nat_action_info); > > + > > + if (*create_new_conn == false) { > > + handle_ftp_ctl(ct, ctx, pkt, conn, now, > CT_FTP_CTL_OTHER, > > + !!nat_action_info); > > + } > > } > > return true; > > } > > -- > > 1.9.1 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/lib/conntrack.c b/lib/conntrack.c index e1c1f2e..b818584 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -1159,8 +1159,11 @@ conn_update_state_alg(struct conntrack *ct, struct dp_packet *pkt, } else { *create_new_conn = conn_update_state(ct, pkt, ctx, &conn, now, bucket); - handle_ftp_ctl(ct, ctx, pkt, conn, now, CT_FTP_CTL_OTHER, - !!nat_action_info); + + if (*create_new_conn == false) { + handle_ftp_ctl(ct, ctx, pkt, conn, now, CT_FTP_CTL_OTHER, + !!nat_action_info); + } } return true; }
Fixes: bd5e81a0e596 ("Userspace Datapath: Add ALG infra and FTP.") Signed-off-by: Darrell Ball <dlu998@gmail.com> --- Needs backporting as far back as 2.8. lib/conntrack.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)