Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1739849 > unrolled thread

[PATCH] ebtables: fix race condition in frame_filter_net_init()

Started byArtem Savkov <asavkov@redhat.com>
First post2017-09-26 14:40 +0200
Last post2017-09-29 13:40 +0200
Articles 6 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH] ebtables: fix race condition in frame_filter_net_init() Artem Savkov <asavkov@redhat.com> - 2017-09-26 14:40 +0200
    Re: [PATCH] ebtables: fix race condition in frame_filter_net_init() Florian Westphal <fw@strlen.de> - 2017-09-26 14:50 +0200
      Re: [PATCH] ebtables: fix race condition in frame_filter_net_init() Artem Savkov <asavkov@redhat.com> - 2017-09-26 16:40 +0200
      [PATCH v2] ebtables: fix race condition in frame_filter_net_init() Artem Savkov <asavkov@redhat.com> - 2017-09-26 17:50 +0200
        [PATCH v3] ebtables: fix race condition in frame_filter_net_init() Artem Savkov <asavkov@redhat.com> - 2017-09-26 18:40 +0200
          Re: [PATCH v3] ebtables: fix race condition in  frame_filter_net_init() Pablo Neira Ayuso <pablo@netfilter.org> - 2017-09-29 13:40 +0200

#1739849 — [PATCH] ebtables: fix race condition in frame_filter_net_init()

FromArtem Savkov <asavkov@redhat.com>
Date2017-09-26 14:40 +0200
Subject[PATCH] ebtables: fix race condition in frame_filter_net_init()
Message-ID<utXjA-4On-27@gated-at.bofh.it>
It is possible for ebt_in_hook to be triggered before ebt_table is assigned
resulting in a NULL-pointer dereference. Make sure hooks are
registered as the last step.

Fixes: aee12a0a3727 ebtables: remove nf_hook_register usage
Signed-off-by: Artem Savkov <asavkov@redhat.com>
---
 include/linux/netfilter_bridge/ebtables.h |  5 +++--
 net/bridge/netfilter/ebtable_broute.c     |  2 +-
 net/bridge/netfilter/ebtable_filter.c     |  8 ++++++--
 net/bridge/netfilter/ebtable_nat.c        |  8 ++++++--
 net/bridge/netfilter/ebtables.c           | 24 +++++++++++++-----------
 5 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/include/linux/netfilter_bridge/ebtables.h b/include/linux/netfilter_bridge/ebtables.h
index 2c2a5514b0df..7d68f5ba6ded 100644
--- a/include/linux/netfilter_bridge/ebtables.h
+++ b/include/linux/netfilter_bridge/ebtables.h
@@ -109,8 +109,9 @@ struct ebt_table {
 #define EBT_ALIGN(s) (((s) + (__alignof__(struct _xt_align)-1)) & \
 		     ~(__alignof__(struct _xt_align)-1))
 extern struct ebt_table *ebt_register_table(struct net *net,
-					    const struct ebt_table *table,
-					    const struct nf_hook_ops *);
+					    const struct ebt_table *table);
+extern int ebt_register_hooks(struct net *net, struct ebt_table *table,
+				 const struct nf_hook_ops *ops);
 extern void ebt_unregister_table(struct net *net, struct ebt_table *table,
 				 const struct nf_hook_ops *);
 extern unsigned int ebt_do_table(struct sk_buff *skb,
diff --git a/net/bridge/netfilter/ebtable_broute.c b/net/bridge/netfilter/ebtable_broute.c
index 2585b100ebbb..b41017409aa5 100644
--- a/net/bridge/netfilter/ebtable_broute.c
+++ b/net/bridge/netfilter/ebtable_broute.c
@@ -65,7 +65,7 @@ static int ebt_broute(struct sk_buff *skb)
 
 static int __net_init broute_net_init(struct net *net)
 {
-	net->xt.broute_table = ebt_register_table(net, &broute_table, NULL);
+	net->xt.broute_table = ebt_register_table(net, &broute_table);
 	return PTR_ERR_OR_ZERO(net->xt.broute_table);
 }
 
diff --git a/net/bridge/netfilter/ebtable_filter.c b/net/bridge/netfilter/ebtable_filter.c
index 45a00dbdbcad..ca04582b374e 100644
--- a/net/bridge/netfilter/ebtable_filter.c
+++ b/net/bridge/netfilter/ebtable_filter.c
@@ -93,8 +93,12 @@ static const struct nf_hook_ops ebt_ops_filter[] = {
 
 static int __net_init frame_filter_net_init(struct net *net)
 {
-	net->xt.frame_filter = ebt_register_table(net, &frame_filter, ebt_ops_filter);
-	return PTR_ERR_OR_ZERO(net->xt.frame_filter);
+	net->xt.frame_filter = ebt_register_table(net, &frame_filter);
+
+	if (IS_ERR(net->xt.frame_filter))
+		return PTR_ERR(net->xt.frame_filter);
+
+	return ebt_register_hooks(net, net->xt.frame_filter, ebt_ops_filter);
 }
 
 static void __net_exit frame_filter_net_exit(struct net *net)
diff --git a/net/bridge/netfilter/ebtable_nat.c b/net/bridge/netfilter/ebtable_nat.c
index 57cd5bb154e7..f4a2ff93be34 100644
--- a/net/bridge/netfilter/ebtable_nat.c
+++ b/net/bridge/netfilter/ebtable_nat.c
@@ -93,8 +93,12 @@ static const struct nf_hook_ops ebt_ops_nat[] = {
 
 static int __net_init frame_nat_net_init(struct net *net)
 {
-	net->xt.frame_nat = ebt_register_table(net, &frame_nat, ebt_ops_nat);
-	return PTR_ERR_OR_ZERO(net->xt.frame_nat);
+	net->xt.frame_nat = ebt_register_table(net, &frame_nat);
+
+	if (IS_ERR(net->xt.frame_nat))
+		return PTR_ERR(net->xt.frame_nat);
+
+	return ebt_register_hooks(net, net->xt.frame_nat, ebt_ops_nat);
 }
 
 static void __net_exit frame_nat_net_exit(struct net *net)
diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 83951f978445..e72120ac426e 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -1169,9 +1169,19 @@ static void __ebt_unregister_table(struct net *net, struct ebt_table *table)
 	kfree(table);
 }
 
+int ebt_register_hooks(struct net *net, struct ebt_table *table,
+		      const struct nf_hook_ops *ops)
+{
+	int ret = nf_register_net_hooks(net, ops, hweight32(table->valid_hooks));
+
+	if (ret)
+		__ebt_unregister_table(net, table);
+
+	return ret;
+}
+
 struct ebt_table *
-ebt_register_table(struct net *net, const struct ebt_table *input_table,
-		   const struct nf_hook_ops *ops)
+ebt_register_table(struct net *net, const struct ebt_table *input_table)
 {
 	struct ebt_table_info *newinfo;
 	struct ebt_table *t, *table;
@@ -1252,15 +1262,6 @@ ebt_register_table(struct net *net, const struct ebt_table *input_table,
 	list_add(&table->list, &net->xt.tables[NFPROTO_BRIDGE]);
 	mutex_unlock(&ebt_mutex);
 
-	if (!ops)
-		return table;
-
-	ret = nf_register_net_hooks(net, ops, hweight32(table->valid_hooks));
-	if (ret) {
-		__ebt_unregister_table(net, table);
-		return ERR_PTR(ret);
-	}
-
 	return table;
 free_unlock:
 	mutex_unlock(&ebt_mutex);
@@ -2456,6 +2457,7 @@ static void __exit ebtables_fini(void)
 	printk(KERN_INFO "Ebtables v2.0 unregistered\n");
 }
 
+EXPORT_SYMBOL(ebt_register_hooks);
 EXPORT_SYMBOL(ebt_register_table);
 EXPORT_SYMBOL(ebt_unregister_table);
 EXPORT_SYMBOL(ebt_do_table);
-- 
2.13.5

[toc] | [next] | [standalone]


#1739853

FromFlorian Westphal <fw@strlen.de>
Date2017-09-26 14:50 +0200
Message-ID<utXtf-4RE-7@gated-at.bofh.it>
In reply to#1739849
Artem Savkov <asavkov@redhat.com> wrote:
> It is possible for ebt_in_hook to be triggered before ebt_table is assigned
> resulting in a NULL-pointer dereference. Make sure hooks are
> registered as the last step.

Right, thanks for the patch.

> --- a/net/bridge/netfilter/ebtable_broute.c
> +++ b/net/bridge/netfilter/ebtable_broute.c
> @@ -65,7 +65,7 @@ static int ebt_broute(struct sk_buff *skb)
>  
>  static int __net_init broute_net_init(struct net *net)
>  {
> -	net->xt.broute_table = ebt_register_table(net, &broute_table, NULL);
> +	net->xt.broute_table = ebt_register_table(net, &broute_table);

I wonder if it makes more sense to model this like the iptables version,
i.e. pass net->xt.table_name as last arg to ebt_register_table ...

> +int ebt_register_hooks(struct net *net, struct ebt_table *table,
> +		      const struct nf_hook_ops *ops)
> +{
> +	int ret = nf_register_net_hooks(net, ops, hweight32(table->valid_hooks));
> +
> +	if (ret)
> +		__ebt_unregister_table(net, table);
> +
> +	return ret;
> +}

... because this looks strange (unregister of table/not-so-obvious error
unwinding ...)

> @@ -1252,15 +1262,6 @@ ebt_register_table(struct net *net, const struct ebt_table *input_table,
>  	list_add(&table->list, &net->xt.tables[NFPROTO_BRIDGE]);
>  	mutex_unlock(&ebt_mutex);

... here one could then assign the net->xt.table_X pointer, and then do
the hook registration right after.

However i have no strong opinion here.

[toc] | [prev] | [next] | [standalone]


#1739944

FromArtem Savkov <asavkov@redhat.com>
Date2017-09-26 16:40 +0200
Message-ID<utZbI-5XL-5@gated-at.bofh.it>
In reply to#1739853
On Tue, Sep 26, 2017 at 02:42:11PM +0200, Florian Westphal wrote:
> Artem Savkov <asavkov@redhat.com> wrote:
> > It is possible for ebt_in_hook to be triggered before ebt_table is assigned
> > resulting in a NULL-pointer dereference. Make sure hooks are
> > registered as the last step.
> 
> Right, thanks for the patch.
> 
> > --- a/net/bridge/netfilter/ebtable_broute.c
> > +++ b/net/bridge/netfilter/ebtable_broute.c
> > @@ -65,7 +65,7 @@ static int ebt_broute(struct sk_buff *skb)
> >  
> >  static int __net_init broute_net_init(struct net *net)
> >  {
> > -	net->xt.broute_table = ebt_register_table(net, &broute_table, NULL);
> > +	net->xt.broute_table = ebt_register_table(net, &broute_table);
> 
> I wonder if it makes more sense to model this like the iptables version,
> i.e. pass net->xt.table_name as last arg to ebt_register_table ...
> 
> > +int ebt_register_hooks(struct net *net, struct ebt_table *table,
> > +		      const struct nf_hook_ops *ops)
> > +{
> > +	int ret = nf_register_net_hooks(net, ops, hweight32(table->valid_hooks));
> > +
> > +	if (ret)
> > +		__ebt_unregister_table(net, table);
> > +
> > +	return ret;
> > +}
> 
> ... because this looks strange (unregister of table/not-so-obvious error
> unwinding ...)
> 
> > @@ -1252,15 +1262,6 @@ ebt_register_table(struct net *net, const struct ebt_table *input_table,
> >  	list_add(&table->list, &net->xt.tables[NFPROTO_BRIDGE]);
> >  	mutex_unlock(&ebt_mutex);
> 
> ... here one could then assign the net->xt.table_X pointer, and then do
> the hook registration right after.
> 
> However i have no strong opinion here.

Agreed, that does look better and requires less changes. I'll send a v2.

-- 
Regards,
  Artem

[toc] | [prev] | [next] | [standalone]


#1739994 — [PATCH v2] ebtables: fix race condition in frame_filter_net_init()

FromArtem Savkov <asavkov@redhat.com>
Date2017-09-26 17:50 +0200
Subject[PATCH v2] ebtables: fix race condition in frame_filter_net_init()
Message-ID<uu0hs-6CT-15@gated-at.bofh.it>
In reply to#1739853
It is possible for ebt_in_hook to be triggered before ebt_table is assigned
resulting in a NULL-pointer dereference. Make sure hooks are
registered as the last step.

Fixes: aee12a0a3727 ebtables: remove nf_hook_register usage
Signed-off-by: Artem Savkov <asavkov@redhat.com>
---
 include/linux/netfilter_bridge/ebtables.h |  7 ++++---
 net/bridge/netfilter/ebtable_broute.c     |  4 ++--
 net/bridge/netfilter/ebtable_filter.c     |  4 ++--
 net/bridge/netfilter/ebtable_nat.c        |  4 ++--
 net/bridge/netfilter/ebtables.c           | 17 ++++++++---------
 5 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/include/linux/netfilter_bridge/ebtables.h b/include/linux/netfilter_bridge/ebtables.h
index 2c2a5514b0df..528b24c78308 100644
--- a/include/linux/netfilter_bridge/ebtables.h
+++ b/include/linux/netfilter_bridge/ebtables.h
@@ -108,9 +108,10 @@ struct ebt_table {
 
 #define EBT_ALIGN(s) (((s) + (__alignof__(struct _xt_align)-1)) & \
 		     ~(__alignof__(struct _xt_align)-1))
-extern struct ebt_table *ebt_register_table(struct net *net,
-					    const struct ebt_table *table,
-					    const struct nf_hook_ops *);
+extern int ebt_register_table(struct net *net,
+			      const struct ebt_table *table,
+			      const struct nf_hook_ops *ops,
+			      struct ebt_table **res);
 extern void ebt_unregister_table(struct net *net, struct ebt_table *table,
 				 const struct nf_hook_ops *);
 extern unsigned int ebt_do_table(struct sk_buff *skb,
diff --git a/net/bridge/netfilter/ebtable_broute.c b/net/bridge/netfilter/ebtable_broute.c
index 2585b100ebbb..276b60262981 100644
--- a/net/bridge/netfilter/ebtable_broute.c
+++ b/net/bridge/netfilter/ebtable_broute.c
@@ -65,8 +65,8 @@ static int ebt_broute(struct sk_buff *skb)
 
 static int __net_init broute_net_init(struct net *net)
 {
-	net->xt.broute_table = ebt_register_table(net, &broute_table, NULL);
-	return PTR_ERR_OR_ZERO(net->xt.broute_table);
+	return ebt_register_table(net, &broute_table, NULL,
+				  &net->xt.broute_table);
 }
 
 static void __net_exit broute_net_exit(struct net *net)
diff --git a/net/bridge/netfilter/ebtable_filter.c b/net/bridge/netfilter/ebtable_filter.c
index 45a00dbdbcad..c41da5fac84f 100644
--- a/net/bridge/netfilter/ebtable_filter.c
+++ b/net/bridge/netfilter/ebtable_filter.c
@@ -93,8 +93,8 @@ static const struct nf_hook_ops ebt_ops_filter[] = {
 
 static int __net_init frame_filter_net_init(struct net *net)
 {
-	net->xt.frame_filter = ebt_register_table(net, &frame_filter, ebt_ops_filter);
-	return PTR_ERR_OR_ZERO(net->xt.frame_filter);
+	return ebt_register_table(net, &frame_filter, ebt_ops_filter,
+				  &net->xt.frame_filter);
 }
 
 static void __net_exit frame_filter_net_exit(struct net *net)
diff --git a/net/bridge/netfilter/ebtable_nat.c b/net/bridge/netfilter/ebtable_nat.c
index 57cd5bb154e7..08df7406ecb3 100644
--- a/net/bridge/netfilter/ebtable_nat.c
+++ b/net/bridge/netfilter/ebtable_nat.c
@@ -93,8 +93,8 @@ static const struct nf_hook_ops ebt_ops_nat[] = {
 
 static int __net_init frame_nat_net_init(struct net *net)
 {
-	net->xt.frame_nat = ebt_register_table(net, &frame_nat, ebt_ops_nat);
-	return PTR_ERR_OR_ZERO(net->xt.frame_nat);
+	return ebt_register_table(net, &frame_nat, ebt_ops_nat,
+				  &net->xt.frame_nat);
 }
 
 static void __net_exit frame_nat_net_exit(struct net *net)
diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 83951f978445..aa81afe81f23 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -1169,9 +1169,8 @@ static void __ebt_unregister_table(struct net *net, struct ebt_table *table)
 	kfree(table);
 }
 
-struct ebt_table *
-ebt_register_table(struct net *net, const struct ebt_table *input_table,
-		   const struct nf_hook_ops *ops)
+int ebt_register_table(struct net *net, const struct ebt_table *input_table,
+		       const struct nf_hook_ops *ops, struct ebt_table **res)
 {
 	struct ebt_table_info *newinfo;
 	struct ebt_table *t, *table;
@@ -1183,7 +1182,7 @@ ebt_register_table(struct net *net, const struct ebt_table *input_table,
 	    repl->entries == NULL || repl->entries_size == 0 ||
 	    repl->counters != NULL || input_table->private != NULL) {
 		BUGPRINT("Bad table data for ebt_register_table!!!\n");
-		return ERR_PTR(-EINVAL);
+		return -EINVAL;
 	}
 
 	/* Don't add one table to multiple lists. */
@@ -1252,16 +1251,16 @@ ebt_register_table(struct net *net, const struct ebt_table *input_table,
 	list_add(&table->list, &net->xt.tables[NFPROTO_BRIDGE]);
 	mutex_unlock(&ebt_mutex);
 
-	if (!ops)
-		return table;
+	WRITE_ONCE(*res, table);
 
 	ret = nf_register_net_hooks(net, ops, hweight32(table->valid_hooks));
 	if (ret) {
 		__ebt_unregister_table(net, table);
-		return ERR_PTR(ret);
+		*res = NULL;
+		return ret;
 	}
 
-	return table;
+	return 0;
 free_unlock:
 	mutex_unlock(&ebt_mutex);
 free_chainstack:
@@ -1276,7 +1275,7 @@ ebt_register_table(struct net *net, const struct ebt_table *input_table,
 free_table:
 	kfree(table);
 out:
-	return ERR_PTR(ret);
+	return ret;
 }
 
 void ebt_unregister_table(struct net *net, struct ebt_table *table,
-- 
2.13.5

[toc] | [prev] | [next] | [standalone]


#1740017 — [PATCH v3] ebtables: fix race condition in frame_filter_net_init()

FromArtem Savkov <asavkov@redhat.com>
Date2017-09-26 18:40 +0200
Subject[PATCH v3] ebtables: fix race condition in frame_filter_net_init()
Message-ID<uu13Q-7af-7@gated-at.bofh.it>
In reply to#1739994
It is possible for ebt_in_hook to be triggered before ebt_table is assigned
resulting in a NULL-pointer dereference. Make sure hooks are
registered as the last step.

v3: restore errorneously removed ops == NULL case check

Fixes: aee12a0a3727 ebtables: remove nf_hook_register usage
Signed-off-by: Artem Savkov <asavkov@redhat.com>
---
 include/linux/netfilter_bridge/ebtables.h |  7 ++++---
 net/bridge/netfilter/ebtable_broute.c     |  4 ++--
 net/bridge/netfilter/ebtable_filter.c     |  4 ++--
 net/bridge/netfilter/ebtable_nat.c        |  4 ++--
 net/bridge/netfilter/ebtables.c           | 17 +++++++++--------
 5 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/include/linux/netfilter_bridge/ebtables.h b/include/linux/netfilter_bridge/ebtables.h
index 2c2a5514b0df..528b24c78308 100644
--- a/include/linux/netfilter_bridge/ebtables.h
+++ b/include/linux/netfilter_bridge/ebtables.h
@@ -108,9 +108,10 @@ struct ebt_table {
 
 #define EBT_ALIGN(s) (((s) + (__alignof__(struct _xt_align)-1)) & \
 		     ~(__alignof__(struct _xt_align)-1))
-extern struct ebt_table *ebt_register_table(struct net *net,
-					    const struct ebt_table *table,
-					    const struct nf_hook_ops *);
+extern int ebt_register_table(struct net *net,
+			      const struct ebt_table *table,
+			      const struct nf_hook_ops *ops,
+			      struct ebt_table **res);
 extern void ebt_unregister_table(struct net *net, struct ebt_table *table,
 				 const struct nf_hook_ops *);
 extern unsigned int ebt_do_table(struct sk_buff *skb,
diff --git a/net/bridge/netfilter/ebtable_broute.c b/net/bridge/netfilter/ebtable_broute.c
index 2585b100ebbb..276b60262981 100644
--- a/net/bridge/netfilter/ebtable_broute.c
+++ b/net/bridge/netfilter/ebtable_broute.c
@@ -65,8 +65,8 @@ static int ebt_broute(struct sk_buff *skb)
 
 static int __net_init broute_net_init(struct net *net)
 {
-	net->xt.broute_table = ebt_register_table(net, &broute_table, NULL);
-	return PTR_ERR_OR_ZERO(net->xt.broute_table);
+	return ebt_register_table(net, &broute_table, NULL,
+				  &net->xt.broute_table);
 }
 
 static void __net_exit broute_net_exit(struct net *net)
diff --git a/net/bridge/netfilter/ebtable_filter.c b/net/bridge/netfilter/ebtable_filter.c
index 45a00dbdbcad..c41da5fac84f 100644
--- a/net/bridge/netfilter/ebtable_filter.c
+++ b/net/bridge/netfilter/ebtable_filter.c
@@ -93,8 +93,8 @@ static const struct nf_hook_ops ebt_ops_filter[] = {
 
 static int __net_init frame_filter_net_init(struct net *net)
 {
-	net->xt.frame_filter = ebt_register_table(net, &frame_filter, ebt_ops_filter);
-	return PTR_ERR_OR_ZERO(net->xt.frame_filter);
+	return ebt_register_table(net, &frame_filter, ebt_ops_filter,
+				  &net->xt.frame_filter);
 }
 
 static void __net_exit frame_filter_net_exit(struct net *net)
diff --git a/net/bridge/netfilter/ebtable_nat.c b/net/bridge/netfilter/ebtable_nat.c
index 57cd5bb154e7..08df7406ecb3 100644
--- a/net/bridge/netfilter/ebtable_nat.c
+++ b/net/bridge/netfilter/ebtable_nat.c
@@ -93,8 +93,8 @@ static const struct nf_hook_ops ebt_ops_nat[] = {
 
 static int __net_init frame_nat_net_init(struct net *net)
 {
-	net->xt.frame_nat = ebt_register_table(net, &frame_nat, ebt_ops_nat);
-	return PTR_ERR_OR_ZERO(net->xt.frame_nat);
+	return ebt_register_table(net, &frame_nat, ebt_ops_nat,
+				  &net->xt.frame_nat);
 }
 
 static void __net_exit frame_nat_net_exit(struct net *net)
diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 83951f978445..3b3dcf719e07 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -1169,9 +1169,8 @@ static void __ebt_unregister_table(struct net *net, struct ebt_table *table)
 	kfree(table);
 }
 
-struct ebt_table *
-ebt_register_table(struct net *net, const struct ebt_table *input_table,
-		   const struct nf_hook_ops *ops)
+int ebt_register_table(struct net *net, const struct ebt_table *input_table,
+		       const struct nf_hook_ops *ops, struct ebt_table **res)
 {
 	struct ebt_table_info *newinfo;
 	struct ebt_table *t, *table;
@@ -1183,7 +1182,7 @@ ebt_register_table(struct net *net, const struct ebt_table *input_table,
 	    repl->entries == NULL || repl->entries_size == 0 ||
 	    repl->counters != NULL || input_table->private != NULL) {
 		BUGPRINT("Bad table data for ebt_register_table!!!\n");
-		return ERR_PTR(-EINVAL);
+		return -EINVAL;
 	}
 
 	/* Don't add one table to multiple lists. */
@@ -1252,16 +1251,18 @@ ebt_register_table(struct net *net, const struct ebt_table *input_table,
 	list_add(&table->list, &net->xt.tables[NFPROTO_BRIDGE]);
 	mutex_unlock(&ebt_mutex);
 
+	WRITE_ONCE(*res, table);
+
 	if (!ops)
-		return table;
+		return 0;
 
 	ret = nf_register_net_hooks(net, ops, hweight32(table->valid_hooks));
 	if (ret) {
 		__ebt_unregister_table(net, table);
-		return ERR_PTR(ret);
+		*res = NULL;
 	}
 
-	return table;
+	return ret;
 free_unlock:
 	mutex_unlock(&ebt_mutex);
 free_chainstack:
@@ -1276,7 +1277,7 @@ ebt_register_table(struct net *net, const struct ebt_table *input_table,
 free_table:
 	kfree(table);
 out:
-	return ERR_PTR(ret);
+	return ret;
 }
 
 void ebt_unregister_table(struct net *net, struct ebt_table *table,
-- 
2.13.5

[toc] | [prev] | [next] | [standalone]


#1742061 — Re: [PATCH v3] ebtables: fix race condition in frame_filter_net_init()

FromPablo Neira Ayuso <pablo@netfilter.org>
Date2017-09-29 13:40 +0200
SubjectRe: [PATCH v3] ebtables: fix race condition in frame_filter_net_init()
Message-ID<uv1Ob-5yp-37@gated-at.bofh.it>
In reply to#1740017
On Tue, Sep 26, 2017 at 06:35:45PM +0200, Artem Savkov wrote:
> It is possible for ebt_in_hook to be triggered before ebt_table is assigned
> resulting in a NULL-pointer dereference. Make sure hooks are
> registered as the last step.

Applied, thanks.

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web