| Message ID | 20190206125111.5286-3-liuhangbin@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | fix two kernel panics when disabled IPv6 on boot up | expand |
On Wed, 6 Feb 2019 20:51:11 +0800 Hangbin Liu <liuhangbin@gmail.com> wrote: > If we disabled IPv6 from kernel boot up cmd(ipv6.disable=1), we should not > call ip6_err_gen_icmpv6_unreach(). > > Reproducer: > ip link add sit1 type sit local 10.10.0.1 remote 10.10.1.1 ttl 1 > ip link set sit1 up > ip addr add 192.168.0.1/24 dev sit1 > ping 192.168.0.2 > > Reported-by: Jianlin Shi <jishi@redhat.com> > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> > --- > net/ipv6/sit.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c > index 1e03305c0549..e43fbac0fd1a 100644 > --- a/net/ipv6/sit.c > +++ b/net/ipv6/sit.c > @@ -493,6 +493,7 @@ static int ipip6_err(struct sk_buff *skb, u32 info) > const int type = icmp_hdr(skb)->type; > const int code = icmp_hdr(skb)->code; > unsigned int data_len = 0; > + struct inet6_dev *idev; > struct ip_tunnel *t; > int sifindex; > int err; > @@ -546,8 +547,13 @@ static int ipip6_err(struct sk_buff *skb, u32 info) > } > > err = 0; > - if (!ip6_err_gen_icmpv6_unreach(skb, iph->ihl * 4, type, data_len)) > + > + idev = in6_dev_get(skb->dev); > + if (idev && > + !ip6_err_gen_icmpv6_unreach(skb, iph->ihl * 4, type, data_len)) { > + in6_dev_put(idev); > goto out; > + } You are leaking a reference if idev && ip6_err_gen_icmpv6_unreach(...). You should call in6_dev_put(idev) whenever idev is not NULL instead.
On 02/06/2019 04:51 AM, Hangbin Liu wrote: > If we disabled IPv6 from kernel boot up cmd(ipv6.disable=1), we should not > call ip6_err_gen_icmpv6_unreach(). > > Reproducer: > ip link add sit1 type sit local 10.10.0.1 remote 10.10.1.1 ttl 1 > ip link set sit1 up > ip addr add 192.168.0.1/24 dev sit1 > ping 192.168.0.2 > > Reported-by: Jianlin Shi <jishi@redhat.com> > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> > --- > net/ipv6/sit.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c > index 1e03305c0549..e43fbac0fd1a 100644 > --- a/net/ipv6/sit.c > +++ b/net/ipv6/sit.c > @@ -493,6 +493,7 @@ static int ipip6_err(struct sk_buff *skb, u32 info) > const int type = icmp_hdr(skb)->type; > const int code = icmp_hdr(skb)->code; > unsigned int data_len = 0; > + struct inet6_dev *idev; > struct ip_tunnel *t; > int sifindex; > int err; > @@ -546,8 +547,13 @@ static int ipip6_err(struct sk_buff *skb, u32 info) > } > > err = 0; > - if (!ip6_err_gen_icmpv6_unreach(skb, iph->ihl * 4, type, data_len)) > + > + idev = in6_dev_get(skb->dev); > + if (idev && > + !ip6_err_gen_icmpv6_unreach(skb, iph->ihl * 4, type, data_len)) { > + in6_dev_put(idev); > goto out; > + } > It seems there is a missing in6_dev_put(idev) depending on ip6_err_gen_icmpv6_unreach()() return value ?
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c index 1e03305c0549..e43fbac0fd1a 100644 --- a/net/ipv6/sit.c +++ b/net/ipv6/sit.c @@ -493,6 +493,7 @@ static int ipip6_err(struct sk_buff *skb, u32 info) const int type = icmp_hdr(skb)->type; const int code = icmp_hdr(skb)->code; unsigned int data_len = 0; + struct inet6_dev *idev; struct ip_tunnel *t; int sifindex; int err; @@ -546,8 +547,13 @@ static int ipip6_err(struct sk_buff *skb, u32 info) } err = 0; - if (!ip6_err_gen_icmpv6_unreach(skb, iph->ihl * 4, type, data_len)) + + idev = in6_dev_get(skb->dev); + if (idev && + !ip6_err_gen_icmpv6_unreach(skb, iph->ihl * 4, type, data_len)) { + in6_dev_put(idev); goto out; + } if (t->parms.iph.daddr == 0) goto out;
If we disabled IPv6 from kernel boot up cmd(ipv6.disable=1), we should not call ip6_err_gen_icmpv6_unreach(). Reproducer: ip link add sit1 type sit local 10.10.0.1 remote 10.10.1.1 ttl 1 ip link set sit1 up ip addr add 192.168.0.1/24 dev sit1 ping 192.168.0.2 Reported-by: Jianlin Shi <jishi@redhat.com> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> --- net/ipv6/sit.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)