[ovs-dev] conntrack: Fix using alg_exp_entry out of scope.

Message ID 1531220750-9384-1-git-send-email-i.maximets@samsung.com
State New
Headers show
Series
  • [ovs-dev] conntrack: Fix using alg_exp_entry out of scope.
Related show

Commit Message

Ilya Maximets July 10, 2018, 11:05 a.m.
'alg_exp_entry' is allocated on stack memory, but could be used via
'alg_exp' pointer inside 'write_ct_md' function, i.e. outside its scope.

CC: Darrell Ball <dlu998@gmail.com>
Fixes: bd5e81a0e596 ("Userspace Datapath: Add ALG infra and FTP.")
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
---

 lib/conntrack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Darrell Ball July 10, 2018, 4:17 p.m. | #1
Good, thanks.

Acked-by: Darrell Ball <dlu998@gmail.com>

This needs backporting to 2.8.

I re-ran some SA tools and found some other potential use after free bugs.


On Tue, Jul 10, 2018 at 4:05 AM, Ilya Maximets <i.maximets@samsung.com>
wrote:

> 'alg_exp_entry' is allocated on stack memory, but could be used via
> 'alg_exp' pointer inside 'write_ct_md' function, i.e. outside its scope.
>
> CC: Darrell Ball <dlu998@gmail.com>
> Fixes: bd5e81a0e596 ("Userspace Datapath: Add ALG infra and FTP.")
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> ---
>
>  lib/conntrack.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index 97fd46a..51c1acb 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -1246,9 +1246,9 @@ process_one(struct conntrack *ct, struct dp_packet
> *pkt,
>      }
>
>      const struct alg_exp_node *alg_exp = NULL;
> +    struct alg_exp_node alg_exp_entry;
>
>      if (OVS_UNLIKELY(create_new_conn)) {
> -        struct alg_exp_node alg_exp_entry;
>
>          ct_rwlock_rdlock(&ct->resources_lock);
>          alg_exp = expectation_lookup(&ct->alg_expectations, &ctx->key,
> --
> 2.7.4
>
>
Ben Pfaff July 10, 2018, 8:32 p.m. | #2
Thanks Ilya and Darrell.  I applied this to master and backported as far
as branch-2.8.

On Tue, Jul 10, 2018 at 09:17:42AM -0700, Darrell Ball wrote:
> Good, thanks.
> 
> Acked-by: Darrell Ball <dlu998@gmail.com>
> 
> This needs backporting to 2.8.
> 
> I re-ran some SA tools and found some other potential use after free bugs.
> 
> 
> On Tue, Jul 10, 2018 at 4:05 AM, Ilya Maximets <i.maximets@samsung.com>
> wrote:
> 
> > 'alg_exp_entry' is allocated on stack memory, but could be used via
> > 'alg_exp' pointer inside 'write_ct_md' function, i.e. outside its scope.
> >
> > CC: Darrell Ball <dlu998@gmail.com>
> > Fixes: bd5e81a0e596 ("Userspace Datapath: Add ALG infra and FTP.")
> > Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> > ---
> >
> >  lib/conntrack.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/conntrack.c b/lib/conntrack.c
> > index 97fd46a..51c1acb 100644
> > --- a/lib/conntrack.c
> > +++ b/lib/conntrack.c
> > @@ -1246,9 +1246,9 @@ process_one(struct conntrack *ct, struct dp_packet
> > *pkt,
> >      }
> >
> >      const struct alg_exp_node *alg_exp = NULL;
> > +    struct alg_exp_node alg_exp_entry;
> >
> >      if (OVS_UNLIKELY(create_new_conn)) {
> > -        struct alg_exp_node alg_exp_entry;
> >
> >          ct_rwlock_rdlock(&ct->resources_lock);
> >          alg_exp = expectation_lookup(&ct->alg_expectations, &ctx->key,
> > --
> > 2.7.4
> >
> >

Patch

diff --git a/lib/conntrack.c b/lib/conntrack.c
index 97fd46a..51c1acb 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -1246,9 +1246,9 @@  process_one(struct conntrack *ct, struct dp_packet *pkt,
     }
 
     const struct alg_exp_node *alg_exp = NULL;
+    struct alg_exp_node alg_exp_entry;
 
     if (OVS_UNLIKELY(create_new_conn)) {
-        struct alg_exp_node alg_exp_entry;
 
         ct_rwlock_rdlock(&ct->resources_lock);
         alg_exp = expectation_lookup(&ct->alg_expectations, &ctx->key,