[ovs-dev,v1;,branch,2.8] conntrack: Fix conn_update_state_alg use after free.

Message ID 1531324412-58234-1-git-send-email-dlu998@gmail.com
State Accepted
Headers show
Series
  • [ovs-dev,v1;,branch,2.8] conntrack: Fix conn_update_state_alg use after free.
Related show

Commit Message

Darrell Ball July 11, 2018, 3:53 p.m.
When conn_update_state() returns true, conn has been freed, so skip calling
handle_ftp_ctl() with this conn and instead follow code path for new
connections.

Fixes: bd5e81a0e596 ("Userspace Datapath: Add ALG infra and FTP.")
Signed-off-by: Darrell Ball <dlu998@gmail.com>
---
 lib/conntrack.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

0-day Robot July 11, 2018, 3:59 p.m. | #1
Bleep bloop.  Greetings Darrell Ball, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


git-am:
Failed to merge in the changes.
Patch failed at 0001 conntrack: Fix conn_update_state_alg use after free.
The copy of the patch that failed is found in:
   /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch
When you have resolved this problem, run "git am --resolved".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


Please check this out.  If you feel there has been an error, please email aconole@bytheb.org

Thanks,
0-day Robot
Ben Pfaff July 11, 2018, 5 p.m. | #2
The bot couldn't apply this patch presumably because it tried to apply
it against master.  Maybe the bot should notice version numbers in [] in
subject lines and try against that branch.

Thanks,

Ben.

On Wed, Jul 11, 2018 at 11:59:24AM -0400, 0-day Robot wrote:
> Bleep bloop.  Greetings Darrell Ball, I am a robot and I have tried out your patch.
> Thanks for your contribution.
> 
> I encountered some error that I wasn't expecting.  See the details below.
> 
> 
> git-am:
> Failed to merge in the changes.
> Patch failed at 0001 conntrack: Fix conn_update_state_alg use after free.
> The copy of the patch that failed is found in:
>    /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch
> When you have resolved this problem, run "git am --resolved".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
> 
> 
> Please check this out.  If you feel there has been an error, please email aconole@bytheb.org
> 
> Thanks,
> 0-day Robot
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Aaron Conole July 11, 2018, 7:48 p.m. | #3
Ben Pfaff <blp@ovn.org> writes:

> The bot couldn't apply this patch presumably because it tried to apply
> it against master.  Maybe the bot should notice version numbers in [] in
> subject lines and try against that branch.

Good call.  I'm at a conference at the moment, but will try to get that
integrated this week.  I expect there will be branch-2.10 and earlier.

> Thanks,
>
> Ben.
>
> On Wed, Jul 11, 2018 at 11:59:24AM -0400, 0-day Robot wrote:
>> Bleep bloop.  Greetings Darrell Ball, I am a robot and I have tried
>> out your patch.
>> Thanks for your contribution.
>> 
>> I encountered some error that I wasn't expecting.  See the details below.
>> 
>> 
>> git-am:
>> Failed to merge in the changes.
>> Patch failed at 0001 conntrack: Fix conn_update_state_alg use after free.
>> The copy of the patch that failed is found in:
>>    /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch
>> When you have resolved this problem, run "git am --resolved".
>> If you prefer to skip this patch, run "git am --skip" instead.
>> To restore the original branch and stop patching, run "git am --abort".
>> 
>> 
>> Please check this out.  If you feel there has been an error, please
>> email aconole@bytheb.org
>> 
>> Thanks,
>> 0-day Robot
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Ben Pfaff July 12, 2018, 9:27 p.m. | #4
On Thu, Jul 12, 2018 at 07:21:36AM -0400, Aaron Conole wrote:
> Aaron Conole <aconole@bytheb.org> writes:
> 
> > Ben Pfaff <blp@ovn.org> writes:
> >
> >> The bot couldn't apply this patch presumably because it tried to apply
> >> it against master.  Maybe the bot should notice version numbers in [] in
> >> subject lines and try against that branch.
> >
> > Good call.  I'm at a conference at the moment, but will try to get that
> > integrated this week.  I expect there will be branch-2.10 and earlier.
> 
> I've updated the bot, and it can now detect subject lines of:
> 
>   branch-[0-9]+.[0-9]+
> 
> (where the . above is literal, not any-char).
> 
> When it detects that, it will checkout the branch before working on any
> of the patches.
> 
> That also means it still would have flagged in this case, but in the
> future if we all use the branch name exactly, it shouldn't give a bad
> flag like this any longer.

Thanks a lot!

Patch

diff --git a/lib/conntrack.c b/lib/conntrack.c
index 56b5cd8..b6d1464 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -1135,8 +1135,11 @@  process_one(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) {
+                    handle_ftp_ctl(ct, ctx, pkt, conn, now, CT_FTP_CTL_OTHER,
+                                   !!nat_action_info);
+                }
             }
         } else {
             create_new_conn = conn_update_state(ct, pkt, ctx, &conn, now,