| Message ID | 1378559600-15858-1-git-send-email-dborkman@redhat.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On Sat, Sep 07, 2013 at 03:13:20PM +0200, Daniel Borkmann wrote: > When the kernel is compiled with CONFIG_IPV6_SUBTREES, and we return > with an error in fn = fib6_add_1(), then error codes are encoded into > the return pointer e.g. ERR_PTR(-ENOENT). In such an error case, we > write the error code into err and jump to out, hence enter the if(err) > condition. Now, if CONFIG_IPV6_SUBTREES is enabled, we check for: > > if (pn != fn && pn->leaf == rt) > ... > if (pn != fn && !pn->leaf && !(pn->fn_flags & RTN_RTINFO)) > ... > > Since pn is NULL and fn is f.e. ERR_PTR(-ENOENT), then pn != fn > evaluates to true and causes a NULL-pointer dereference on further > checks on pn. Fix it, by setting both NULL in error case, so that > pn != fn already evaluates to false and no further dereference > takes place. > > This was first correctly implemented in 4a287eba2 ("IPv6 routing, > NLM_F_* flag support: REPLACE and EXCL flags support, warn about > missing CREATE flag"), but the bug got later on introduced by > 188c517a0 ("ipv6: return errno pointers consistently for fib6_add_1()"). > > Signed-off-by: Daniel Borkmann <dborkman@redhat.com> > Cc: Lin Ming <mlin@ss.pku.edu.cn> > Cc: Matti Vaittinen <matti.vaittinen@nsn.com> > Cc: Hannes Frederic Sowa <hannes@stressinduktion.org> Full ACK! Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/07/2013 10:35 PM, ext Hannes Frederic Sowa wrote: > On Sat, Sep 07, 2013 at 03:13:20PM +0200, Daniel Borkmann wrote: >> When the kernel is compiled with CONFIG_IPV6_SUBTREES, and we return >> with an error in fn = fib6_add_1(), then error codes are encoded into >> the return pointer e.g. ERR_PTR(-ENOENT). In such an error case, we >> write the error code into err and jump to out, hence enter the if(err) >> condition. Now, if CONFIG_IPV6_SUBTREES is enabled, we check for: >> >> if (pn != fn && pn->leaf == rt) >> ... >> if (pn != fn && !pn->leaf && !(pn->fn_flags & RTN_RTINFO)) >> ... >> >> Since pn is NULL and fn is f.e. ERR_PTR(-ENOENT), then pn != fn >> evaluates to true and causes a NULL-pointer dereference on further >> checks on pn. Fix it, by setting both NULL in error case, so that >> pn != fn already evaluates to false and no further dereference >> takes place. >> >> This was first correctly implemented in 4a287eba2 ("IPv6 routing, >> NLM_F_* flag support: REPLACE and EXCL flags support, warn about >> missing CREATE flag"), but the bug got later on introduced by >> 188c517a0 ("ipv6: return errno pointers consistently for fib6_add_1()"). >> >> Signed-off-by: Daniel Borkmann <dborkman@redhat.com> >> Cc: Lin Ming <mlin@ss.pku.edu.cn> >> Cc: Matti Vaittinen <matti.vaittinen@nsn.com> >> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org> > > Full ACK! > > Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org> > Acked-by: Matti Vaittinen <matti.vaittinen@nsn.com> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Matti Vaittinen <matti.vaittinen@nsn.com> Date: Mon, 09 Sep 2013 09:26:07 +0300 > On 09/07/2013 10:35 PM, ext Hannes Frederic Sowa wrote: >> On Sat, Sep 07, 2013 at 03:13:20PM +0200, Daniel Borkmann wrote: >>> When the kernel is compiled with CONFIG_IPV6_SUBTREES, and we return >>> with an error in fn = fib6_add_1(), then error codes are encoded into >>> the return pointer e.g. ERR_PTR(-ENOENT). In such an error case, we >>> write the error code into err and jump to out, hence enter the if(err) >>> condition. Now, if CONFIG_IPV6_SUBTREES is enabled, we check for: >>> >>> if (pn != fn && pn->leaf == rt) >>> ... >>> if (pn != fn && !pn->leaf && !(pn->fn_flags & RTN_RTINFO)) >>> ... >>> >>> Since pn is NULL and fn is f.e. ERR_PTR(-ENOENT), then pn != fn >>> evaluates to true and causes a NULL-pointer dereference on further >>> checks on pn. Fix it, by setting both NULL in error case, so that >>> pn != fn already evaluates to false and no further dereference >>> takes place. >>> >>> This was first correctly implemented in 4a287eba2 ("IPv6 routing, >>> NLM_F_* flag support: REPLACE and EXCL flags support, warn about >>> missing CREATE flag"), but the bug got later on introduced by >>> 188c517a0 ("ipv6: return errno pointers consistently for >>> fib6_add_1()"). >>> >>> Signed-off-by: Daniel Borkmann <dborkman@redhat.com> >>> Cc: Lin Ming <mlin@ss.pku.edu.cn> >>> Cc: Matti Vaittinen <matti.vaittinen@nsn.com> >>> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org> >> >> Full ACK! >> >> Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org> >> > Acked-by: Matti Vaittinen <matti.vaittinen@nsn.com> Applied, thanks everyone. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c index 73db48e..5bec666 100644 --- a/net/ipv6/ip6_fib.c +++ b/net/ipv6/ip6_fib.c @@ -825,9 +825,9 @@ int fib6_add(struct fib6_node *root, struct rt6_info *rt, struct nl_info *info) fn = fib6_add_1(root, &rt->rt6i_dst.addr, rt->rt6i_dst.plen, offsetof(struct rt6_info, rt6i_dst), allow_create, replace_required); - if (IS_ERR(fn)) { err = PTR_ERR(fn); + fn = NULL; goto out; }
When the kernel is compiled with CONFIG_IPV6_SUBTREES, and we return with an error in fn = fib6_add_1(), then error codes are encoded into the return pointer e.g. ERR_PTR(-ENOENT). In such an error case, we write the error code into err and jump to out, hence enter the if(err) condition. Now, if CONFIG_IPV6_SUBTREES is enabled, we check for: if (pn != fn && pn->leaf == rt) ... if (pn != fn && !pn->leaf && !(pn->fn_flags & RTN_RTINFO)) ... Since pn is NULL and fn is f.e. ERR_PTR(-ENOENT), then pn != fn evaluates to true and causes a NULL-pointer dereference on further checks on pn. Fix it, by setting both NULL in error case, so that pn != fn already evaluates to false and no further dereference takes place. This was first correctly implemented in 4a287eba2 ("IPv6 routing, NLM_F_* flag support: REPLACE and EXCL flags support, warn about missing CREATE flag"), but the bug got later on introduced by 188c517a0 ("ipv6: return errno pointers consistently for fib6_add_1()"). Signed-off-by: Daniel Borkmann <dborkman@redhat.com> Cc: Lin Ming <mlin@ss.pku.edu.cn> Cc: Matti Vaittinen <matti.vaittinen@nsn.com> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org> --- net/ipv6/ip6_fib.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)