* [PATCH net 0/3] ipv6: fix error path of inet6_init()
@ 2018-08-28 11:40 Sabrina Dubroca
2018-08-28 11:40 ` [PATCH net 1/3] ipv6: fix cleanup ordering for ip6_mr failure Sabrina Dubroca
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Sabrina Dubroca @ 2018-08-28 11:40 UTC (permalink / raw)
To: netdev; +Cc: Sabrina Dubroca, Xin Long
The error path of inet6_init() can trigger multiple kernel panics,
mostly due to wrong ordering of cleanups. This series fixes those
issues.
Sabrina Dubroca (3):
ipv6: fix cleanup ordering for ip6_mr failure
ipv6: fix cleanup ordering for pingv6 registration
net: rtnl: return early from rtnl_unregister_all when protocol isn't
registered
net/core/rtnetlink.c | 4 ++++
net/ipv6/af_inet6.c | 10 +++++-----
2 files changed, 9 insertions(+), 5 deletions(-)
--
2.18.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net 1/3] ipv6: fix cleanup ordering for ip6_mr failure
2018-08-28 11:40 [PATCH net 0/3] ipv6: fix error path of inet6_init() Sabrina Dubroca
@ 2018-08-28 11:40 ` Sabrina Dubroca
2018-08-28 11:40 ` [PATCH net 2/3] ipv6: fix cleanup ordering for pingv6 registration Sabrina Dubroca
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Sabrina Dubroca @ 2018-08-28 11:40 UTC (permalink / raw)
To: netdev; +Cc: Sabrina Dubroca, WANG Cong, Xin Long, Andrey Konovalov
Commit 15e668070a64 ("ipv6: reorder icmpv6_init() and ip6_mr_init()")
moved the cleanup label for ipmr_fail, but should have changed the
contents of the cleanup labels as well. Now we can end up cleaning up
icmpv6 even though it hasn't been initialized (jump to icmp_fail or
ipmr_fail).
Simply undo things in the reverse order of their initialization.
Example of panic (triggered by faking a failure of icmpv6_init):
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault: 0000 [#1] PREEMPT SMP KASAN PTI
[...]
RIP: 0010:__list_del_entry_valid+0x79/0x160
[...]
Call Trace:
? lock_release+0x8a0/0x8a0
unregister_pernet_operations+0xd4/0x560
? ops_free_list+0x480/0x480
? down_write+0x91/0x130
? unregister_pernet_subsys+0x15/0x30
? down_read+0x1b0/0x1b0
? up_read+0x110/0x110
? kmem_cache_create_usercopy+0x1b4/0x240
unregister_pernet_subsys+0x1d/0x30
icmpv6_cleanup+0x1d/0x30
inet6_init+0x1b5/0x23f
Fixes: 15e668070a64 ("ipv6: reorder icmpv6_init() and ip6_mr_init()")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
net/ipv6/af_inet6.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index 673bba31eb18..e5da133c6932 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -1113,11 +1113,11 @@ static int __init inet6_init(void)
igmp_fail:
ndisc_cleanup();
ndisc_fail:
- ip6_mr_cleanup();
+ icmpv6_cleanup();
icmp_fail:
- unregister_pernet_subsys(&inet6_net_ops);
+ ip6_mr_cleanup();
ipmr_fail:
- icmpv6_cleanup();
+ unregister_pernet_subsys(&inet6_net_ops);
register_pernet_fail:
sock_unregister(PF_INET6);
rtnl_unregister_all(PF_INET6);
--
2.18.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net 2/3] ipv6: fix cleanup ordering for pingv6 registration
2018-08-28 11:40 [PATCH net 0/3] ipv6: fix error path of inet6_init() Sabrina Dubroca
2018-08-28 11:40 ` [PATCH net 1/3] ipv6: fix cleanup ordering for ip6_mr failure Sabrina Dubroca
@ 2018-08-28 11:40 ` Sabrina Dubroca
2018-08-28 11:40 ` [PATCH net 3/3] net: rtnl: return early from rtnl_unregister_all when protocol isn't registered Sabrina Dubroca
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Sabrina Dubroca @ 2018-08-28 11:40 UTC (permalink / raw)
To: netdev; +Cc: Sabrina Dubroca, Xin Long, Lorenzo Colitti
Commit 6d0bfe226116 ("net: ipv6: Add IPv6 support to the ping socket.")
contains an error in the cleanup path of inet6_init(): when
proto_register(&pingv6_prot, 1) fails, we try to unregister
&pingv6_prot. When rawv6_init() fails, we skip unregistering
&pingv6_prot.
Example of panic (triggered by faking a failure of
proto_register(&pingv6_prot, 1)):
general protection fault: 0000 [#1] PREEMPT SMP KASAN PTI
[...]
RIP: 0010:__list_del_entry_valid+0x79/0x160
[...]
Call Trace:
proto_unregister+0xbb/0x550
? trace_preempt_on+0x6f0/0x6f0
? sock_no_shutdown+0x10/0x10
inet6_init+0x153/0x1b8
Fixes: 6d0bfe226116 ("net: ipv6: Add IPv6 support to the ping socket.")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
net/ipv6/af_inet6.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index e5da133c6932..9a4261e50272 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -938,14 +938,14 @@ static int __init inet6_init(void)
err = proto_register(&pingv6_prot, 1);
if (err)
- goto out_unregister_ping_proto;
+ goto out_unregister_raw_proto;
/* We MUST register RAW sockets before we create the ICMP6,
* IGMP6, or NDISC control sockets.
*/
err = rawv6_init();
if (err)
- goto out_unregister_raw_proto;
+ goto out_unregister_ping_proto;
/* Register the family here so that the init calls below will
* be able to create sockets. (?? is this dangerous ??)
--
2.18.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net 3/3] net: rtnl: return early from rtnl_unregister_all when protocol isn't registered
2018-08-28 11:40 [PATCH net 0/3] ipv6: fix error path of inet6_init() Sabrina Dubroca
2018-08-28 11:40 ` [PATCH net 1/3] ipv6: fix cleanup ordering for ip6_mr failure Sabrina Dubroca
2018-08-28 11:40 ` [PATCH net 2/3] ipv6: fix cleanup ordering for pingv6 registration Sabrina Dubroca
@ 2018-08-28 11:40 ` Sabrina Dubroca
2018-08-28 15:30 ` [PATCH net 0/3] ipv6: fix error path of inet6_init() Xin Long
2018-08-30 2:29 ` David Miller
4 siblings, 0 replies; 6+ messages in thread
From: Sabrina Dubroca @ 2018-08-28 11:40 UTC (permalink / raw)
To: netdev; +Cc: Sabrina Dubroca, Xin Long
rtnl_unregister_all(PF_INET6) gets called from inet6_init in cases when
no handler has been registered for PF_INET6 yet, for example if
ip6_mr_init() fails. Abort and avoid a NULL pointer deref in that case.
Example of panic (triggered by faking a failure of
register_pernet_subsys):
general protection fault: 0000 [#1] PREEMPT SMP KASAN PTI
[...]
RIP: 0010:rtnl_unregister_all+0x17e/0x2a0
[...]
Call Trace:
? rtnetlink_net_init+0x250/0x250
? sock_unregister+0x103/0x160
? kernel_getsockopt+0x200/0x200
inet6_init+0x197/0x20d
Fixes: e2fddf5e96df ("[IPV6]: Make af_inet6 to check ip6_route_init return value.")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
net/core/rtnetlink.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 24431e578310..60c928894a78 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -324,6 +324,10 @@ void rtnl_unregister_all(int protocol)
rtnl_lock();
tab = rtnl_msg_handlers[protocol];
+ if (!tab) {
+ rtnl_unlock();
+ return;
+ }
RCU_INIT_POINTER(rtnl_msg_handlers[protocol], NULL);
for (msgindex = 0; msgindex < RTM_NR_MSGTYPES; msgindex++) {
link = tab[msgindex];
--
2.18.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net 0/3] ipv6: fix error path of inet6_init()
2018-08-28 11:40 [PATCH net 0/3] ipv6: fix error path of inet6_init() Sabrina Dubroca
` (2 preceding siblings ...)
2018-08-28 11:40 ` [PATCH net 3/3] net: rtnl: return early from rtnl_unregister_all when protocol isn't registered Sabrina Dubroca
@ 2018-08-28 15:30 ` Xin Long
2018-08-30 2:29 ` David Miller
4 siblings, 0 replies; 6+ messages in thread
From: Xin Long @ 2018-08-28 15:30 UTC (permalink / raw)
To: Sabrina Dubroca; +Cc: netdev
----- Original Message -----
> The error path of inet6_init() can trigger multiple kernel panics,
> mostly due to wrong ordering of cleanups. This series fixes those
> issues.
>
> Sabrina Dubroca (3):
> ipv6: fix cleanup ordering for ip6_mr failure
> ipv6: fix cleanup ordering for pingv6 registration
> net: rtnl: return early from rtnl_unregister_all when protocol isn't
> registered
>
> net/core/rtnetlink.c | 4 ++++
> net/ipv6/af_inet6.c | 10 +++++-----
> 2 files changed, 9 insertions(+), 5 deletions(-)
>
> --
> 2.18.0
>
>
Series Reviewed-by: Xin Long <lucien.xin@gmail.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 0/3] ipv6: fix error path of inet6_init()
2018-08-28 11:40 [PATCH net 0/3] ipv6: fix error path of inet6_init() Sabrina Dubroca
` (3 preceding siblings ...)
2018-08-28 15:30 ` [PATCH net 0/3] ipv6: fix error path of inet6_init() Xin Long
@ 2018-08-30 2:29 ` David Miller
4 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2018-08-30 2:29 UTC (permalink / raw)
To: sd; +Cc: netdev, lxin
From: Sabrina Dubroca <sd@queasysnail.net>
Date: Tue, 28 Aug 2018 13:40:50 +0200
> The error path of inet6_init() can trigger multiple kernel panics,
> mostly due to wrong ordering of cleanups. This series fixes those
> issues.
Series applied, thank you.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-08-30 6:29 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-28 11:40 [PATCH net 0/3] ipv6: fix error path of inet6_init() Sabrina Dubroca
2018-08-28 11:40 ` [PATCH net 1/3] ipv6: fix cleanup ordering for ip6_mr failure Sabrina Dubroca
2018-08-28 11:40 ` [PATCH net 2/3] ipv6: fix cleanup ordering for pingv6 registration Sabrina Dubroca
2018-08-28 11:40 ` [PATCH net 3/3] net: rtnl: return early from rtnl_unregister_all when protocol isn't registered Sabrina Dubroca
2018-08-28 15:30 ` [PATCH net 0/3] ipv6: fix error path of inet6_init() Xin Long
2018-08-30 2:29 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.