ipv6: Fix finding best source address in ipv6_dev_get_saddr().

Commit 9131f3de2 ("ipv6: Do not iterate over all interfaces when finding source address on specific interface.") did not properly update best source address available. Plus, it introduced possible NULL pointer dereference.

Bug was reported by Erik Kline <[email protected]>.
Based on patch proposed by Hajime Tazaki <[email protected]>.

Fixes 9131f3de24db4dc12199aede7d931e6703e97f3b ("ipv6: Do not

iterate over all interfaces when finding source address on specific interface.")

Signed-off-by YOSHIFUJI Hideaki <[email protected]>

Commit 9131f3de2 ("ipv6: Do not iterate over all interfaces when finding source address on specific interface.") did not properly update best source address available. Plus, it introduced possible NULL pointer dereference.

Bug was reported by Erik Kline <[email protected]>.
Based on patch proposed by Hajime Tazaki <[email protected]>.

Fixes 9131f3de24db4dc12199aede7d931e6703e97f3b ("ipv6: Do not

iterate over all interfaces when finding source address on specific interface.")

Signed-off-by YOSHIFUJI Hideaki <[email protected]>

---
 net/ipv6/addrconf.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

net/ipv6/addrconf.c +18 -13

index 4ab74d5..4c9a024 100644
@@ -1358,14 +1358,15 @@ out:
        return ret;
 }
 
-static void __ipv6_dev_get_saddr(struct net *net,
-                                struct ipv6_saddr_dst *dst,
-                                unsigned int prefs,
-                                const struct in6_addr *saddr,
-                                struct inet6_dev *idev,
-                                struct ipv6_saddr_score *scores)
+static int __ipv6_dev_get_saddr(struct net *net,
+                               struct ipv6_saddr_dst *dst,
+                               unsigned int prefs,
+                               const struct in6_addr *saddr,
+                               struct inet6_dev *idev,
+                               struct ipv6_saddr_score *scores,
+                               int hiscore_idx)
 {
-       struct ipv6_saddr_score *score = &scores[0], *hiscore = &scores[1];
+       struct ipv6_saddr_score *score = &scores[1 - hiscore_idx], *hiscore = &scores[hiscore_idx];
 
        read_lock_bh(&idev->lock);
        list_for_each_entry(score->ifa, &idev->addr_list, if_list) {

@@ -1424,6 +1425,7 @@ static void __ipv6_dev_get_saddr(struct net *net,
                                in6_ifa_hold(score->ifa);
 
                                swap(hiscore, score);
+                               hiscore_idx = 1 - hiscore_idx;
 
                                /* restore our iterator */
                                score->ifa = hiscore->ifa;

@@ -1434,18 +1436,20 @@ static void __ipv6_dev_get_saddr(struct net *net,
        }
 out:
        read_unlock_bh(&idev->lock);
+       return hiscore_idx;
 }
 
 int ipv6_dev_get_saddr(struct net *net, const struct net_device *dst_dev,
                       const struct in6_addr *daddr, unsigned int prefs,
                       struct in6_addr *saddr)
 {
-       struct ipv6_saddr_score scores[2], *hiscore = &scores[1];
+       struct ipv6_saddr_score scores[2], *hiscore;
        struct ipv6_saddr_dst dst;
        struct inet6_dev *idev;
        struct net_device *dev;
        int dst_type;
        bool use_oif_addr = false;
+       int hiscore_idx = 0;
 
        dst_type = __ipv6_addr_type(daddr);
        dst.addr = daddr;

@@ -1454,8 +1458,8 @@ int ipv6_dev_get_saddr(struct net *net, const struct net_device *dst_dev,
        dst.label = ipv6_addr_label(net, daddr, dst_type, dst.ifindex);
        dst.prefs = prefs;
 
-       hiscore->rule = -1;
-       hiscore->ifa = NULL;
+       scores[hiscore_idx].rule = -1;
+       scores[hiscore_idx].ifa = NULL;
 
        rcu_read_lock();
 

@@ -1480,17 +1484,19 @@ int ipv6_dev_get_saddr(struct net *net, const struct net_device *dst_dev,
        }
 
        if (use_oif_addr) {
-               __ipv6_dev_get_saddr(net, &dst, prefs, saddr, idev, scores);
+               if (idev)
+                       hiscore_idx = __ipv6_dev_get_saddr(net, &dst, prefs, saddr, idev, scores, hiscore_idx);
        } else {
                for_each_netdev_rcu(net, dev) {
                        idev = __in6_dev_get(dev);
                        if (!idev)
                                continue;
-                       __ipv6_dev_get_saddr(net, &dst, prefs, saddr, idev, scores);
+                       hiscore_idx = __ipv6_dev_get_saddr(net, &dst, prefs, saddr, idev, scores, hiscore_idx);
                }
        }
        rcu_read_unlock();
 
+       hiscore = &scores[hiscore_idx];
        if (!hiscore->ifa)
                return -EADDRNOTAVAIL;
 
--

1.9.1

--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

I am testing this patch which may be a little simpler. Also idev needs to be checked after __in6_dev_get

Tom

net/ipv6/addrconf.c +11 -6

index 4ab74d5..d631ac3 100644
@@ -1363,9 +1363,10 @@ static void __ipv6_dev_get_saddr(struct net *net,
                                 unsigned int prefs,
                                 const struct in6_addr *saddr,
                                 struct inet6_dev *idev,
-                                struct ipv6_saddr_score *scores)
+                                struct ipv6_saddr_score **in_score,
+                                struct ipv6_saddr_score **in_hiscore)
 {
-       struct ipv6_saddr_score *score = &scores[0], *hiscore = &scores[1];
+       struct ipv6_saddr_score *score = *in_score, *hiscore = *in_hiscore;
 
        read_lock_bh(&idev->lock);
        list_for_each_entry(score->ifa, &idev->addr_list, if_list) {

@@ -1434,13 +1435,16 @@ static void __ipv6_dev_get_saddr(struct net *net,
        }
 out:
        read_unlock_bh(&idev->lock);
+       *in_hiscore = hiscore;
+       *in_score = score;
 }
 
 int ipv6_dev_get_saddr(struct net *net, const struct net_device *dst_dev,
                       const struct in6_addr *daddr, unsigned int prefs,
                       struct in6_addr *saddr)
 {
-       struct ipv6_saddr_score scores[2], *hiscore = &scores[1];
+       struct ipv6_saddr_score scores[2];
+       struct ipv6_saddr_score *score = &scores[0], *hiscore = &scores[1];
        struct ipv6_saddr_dst dst;
        struct inet6_dev *idev;
        struct net_device *dev;

@@ -1475,18 +1479,19 @@ int ipv6_dev_get_saddr(struct net *net, const
 struct net_device *dst_dev,
                if ((dst_type & IPV6_ADDR_MULTICAST) ||
                    dst.scope <= IPV6_ADDR_SCOPE_LINKLOCAL) {
                        idev = __in6_dev_get(dst_dev);
-                       use_oif_addr = true;
+                       if (idev)
+                               use_oif_addr = true;
                }
        }
        if (use_oif_addr) {
-               __ipv6_dev_get_saddr(net, &dst, prefs, saddr, idev, scores);
+               __ipv6_dev_get_saddr(net, &dst, prefs, saddr, idev,
 &score, &hiscore);
        } else {
                for_each_netdev_rcu(net, dev) {
                        idev = __in6_dev_get(dev);
                        if (!idev)
                                continue;
-                       __ipv6_dev_get_saddr(net, &dst, prefs, saddr,
 idev, scores);
+                       __ipv6_dev_get_saddr(net, &dst, prefs, saddr,
 idev, &score, &hiscore);

                }
        }
        rcu_read_unlock();

Quoted 113 lines · YOSHIFUJI Hideaki/吉藤英明

Commit 9131f3de2 ("ipv6: Do not iterate over all interfaces when finding source address on specific interface.") did not properly update best source address available. Plus, it introduced possible NULL pointer dereference.

Bug was reported by Erik Kline <[email protected]>.
Based on patch proposed by Hajime Tazaki <[email protected]>.

Fixes 9131f3de24db4dc12199aede7d931e6703e97f3b ("ipv6: Do not

iterate over all interfaces when finding source address on specific interface.")

Signed-off-by YOSHIFUJI Hideaki <[email protected]>

---
 net/ipv6/addrconf.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

net/ipv6/addrconf.c +18 -13

index 4ab74d5..4c9a024 100644
@@ -1358,14 +1358,15 @@ out:
        return ret;
 }
 
-static void __ipv6_dev_get_saddr(struct net *net,
-                                struct ipv6_saddr_dst *dst,
-                                unsigned int prefs,
-                                const struct in6_addr *saddr,
-                                struct inet6_dev *idev,
-                                struct ipv6_saddr_score *scores)
+static int __ipv6_dev_get_saddr(struct net *net,
+                               struct ipv6_saddr_dst *dst,
+                               unsigned int prefs,
+                               const struct in6_addr *saddr,
+                               struct inet6_dev *idev,
+                               struct ipv6_saddr_score *scores,
+                               int hiscore_idx)
 {
-       struct ipv6_saddr_score *score = &scores[0], *hiscore = &scores[1];
+       struct ipv6_saddr_score *score = &scores[1 - hiscore_idx], *hiscore = &scores[hiscore_idx];
 
        read_lock_bh(&idev->lock);
        list_for_each_entry(score->ifa, &idev->addr_list, if_list) {

@@ -1424,6 +1425,7 @@ static void __ipv6_dev_get_saddr(struct net *net,
                                in6_ifa_hold(score->ifa);
 
                                swap(hiscore, score);
+                               hiscore_idx = 1 - hiscore_idx;
 
                                /* restore our iterator */
                                score->ifa = hiscore->ifa;

@@ -1434,18 +1436,20 @@ static void __ipv6_dev_get_saddr(struct net *net,
        }
 out:
        read_unlock_bh(&idev->lock);
+       return hiscore_idx;
 }
 
 int ipv6_dev_get_saddr(struct net *net, const struct net_device *dst_dev,
                       const struct in6_addr *daddr, unsigned int prefs,
                       struct in6_addr *saddr)
 {
-       struct ipv6_saddr_score scores[2], *hiscore = &scores[1];
+       struct ipv6_saddr_score scores[2], *hiscore;
        struct ipv6_saddr_dst dst;
        struct inet6_dev *idev;
        struct net_device *dev;
        int dst_type;
        bool use_oif_addr = false;
+       int hiscore_idx = 0;
 
        dst_type = __ipv6_addr_type(daddr);
        dst.addr = daddr;

@@ -1454,8 +1458,8 @@ int ipv6_dev_get_saddr(struct net *net, const struct net_device *dst_dev,
        dst.label = ipv6_addr_label(net, daddr, dst_type, dst.ifindex);
        dst.prefs = prefs;
 
-       hiscore->rule = -1;
-       hiscore->ifa = NULL;
+       scores[hiscore_idx].rule = -1;
+       scores[hiscore_idx].ifa = NULL;
 
        rcu_read_lock();
 

@@ -1480,17 +1484,19 @@ int ipv6_dev_get_saddr(struct net *net, const struct net_device *dst_dev,
        }
 
        if (use_oif_addr) {
-               __ipv6_dev_get_saddr(net, &dst, prefs, saddr, idev, scores);
+               if (idev)
+                       hiscore_idx = __ipv6_dev_get_saddr(net, &dst, prefs, saddr, idev, scores, hiscore_idx);
        } else {
                for_each_netdev_rcu(net, dev) {
                        idev = __in6_dev_get(dev);
                        if (!idev)
                                continue;
-                       __ipv6_dev_get_saddr(net, &dst, prefs, saddr, idev, scores);
+                       hiscore_idx = __ipv6_dev_get_saddr(net, &dst, prefs, saddr, idev, scores, hiscore_idx);
                }
        }
        rcu_read_unlock();
 
+       hiscore = &scores[hiscore_idx];
        if (!hiscore->ifa)
                return -EADDRNOTAVAIL;
 
--

1.9.1

--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Hi,

Quoted 2 lines · Tom Herbert

I am testing this patch which may be a little simpler. Also idev needs to be checked after __in6_dev_get

We have to select source address on *given* interface for link-local/ multicast destinations.

Quoted 173 lines

Tom

net/ipv6/addrconf.c +11 -6

index 4ab74d5..d631ac3 100644
@@ -1363,9 +1363,10 @@ static void __ipv6_dev_get_saddr(struct net *net,
                                 unsigned int prefs,
                                 const struct in6_addr *saddr,
                                 struct inet6_dev *idev,
-                                struct ipv6_saddr_score *scores)
+                                struct ipv6_saddr_score **in_score,
+                                struct ipv6_saddr_score **in_hiscore)
 {
-       struct ipv6_saddr_score *score = &scores[0], *hiscore = &scores[1];
+       struct ipv6_saddr_score *score = *in_score, *hiscore = *in_hiscore;
 
        read_lock_bh(&idev->lock);
        list_for_each_entry(score->ifa, &idev->addr_list, if_list) {

@@ -1434,13 +1435,16 @@ static void __ipv6_dev_get_saddr(struct net *net,
        }
 out:
        read_unlock_bh(&idev->lock);
+       *in_hiscore = hiscore;
+       *in_score = score;
 }
 
 int ipv6_dev_get_saddr(struct net *net, const struct net_device *dst_dev,
                       const struct in6_addr *daddr, unsigned int prefs,
                       struct in6_addr *saddr)
 {
-       struct ipv6_saddr_score scores[2], *hiscore = &scores[1];
+       struct ipv6_saddr_score scores[2];
+       struct ipv6_saddr_score *score = &scores[0], *hiscore = &scores[1];
        struct ipv6_saddr_dst dst;
        struct inet6_dev *idev;
        struct net_device *dev;

@@ -1475,18 +1479,19 @@ int ipv6_dev_get_saddr(struct net *net, const
 struct net_device *dst_dev,
                if ((dst_type & IPV6_ADDR_MULTICAST) ||
                    dst.scope <= IPV6_ADDR_SCOPE_LINKLOCAL) {
                        idev = __in6_dev_get(dst_dev);
-                       use_oif_addr = true;
+                       if (idev)
+                               use_oif_addr = true;
                }
        }
        if (use_oif_addr) {
-               __ipv6_dev_get_saddr(net, &dst, prefs, saddr, idev, scores);
+               __ipv6_dev_get_saddr(net, &dst, prefs, saddr, idev,
 &score, &hiscore);
        } else {
                for_each_netdev_rcu(net, dev) {
                        idev = __in6_dev_get(dev);
                        if (!idev)
                                continue;
-                       __ipv6_dev_get_saddr(net, &dst, prefs, saddr,
 idev, scores);
+                       __ipv6_dev_get_saddr(net, &dst, prefs, saddr,
 idev, &score, &hiscore);

                }
        }
        rcu_read_unlock();

Quoted 106 lines · YOSHIFUJI Hideaki/吉藤英明

Commit 9131f3de2 ("ipv6: Do not iterate over all interfaces when finding source address on specific interface.") did not properly update best source address available. Plus, it introduced possible NULL pointer dereference.

Bug was reported by Erik Kline <[email protected]>.
Based on patch proposed by Hajime Tazaki <[email protected]>.

Fixes 9131f3de24db4dc12199aede7d931e6703e97f3b ("ipv6: Do not

iterate over all interfaces when finding source address on specific interface.")

Signed-off-by YOSHIFUJI Hideaki <[email protected]>

---
 net/ipv6/addrconf.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

net/ipv6/addrconf.c +18 -12

index 4ab74d5..4c9a024 100644
@@ -1358,14 +1358,15 @@ out:
        return ret;
 }
 
-static void __ipv6_dev_get_saddr(struct net *net,
-                                struct ipv6_saddr_dst *dst,
-                                unsigned int prefs,
-                                const struct in6_addr *saddr,
-                                struct inet6_dev *idev,
-                                struct ipv6_saddr_score *scores)
+static int __ipv6_dev_get_saddr(struct net *net,
+                               struct ipv6_saddr_dst *dst,
+                               unsigned int prefs,
+                               const struct in6_addr *saddr,
+                               struct inet6_dev *idev,
+                               struct ipv6_saddr_score *scores,
+                               int hiscore_idx)
 {
-       struct ipv6_saddr_score *score = &scores[0], *hiscore = &scores[1];
+       struct ipv6_saddr_score *score = &scores[1 - hiscore_idx], *hiscore = &scores[hiscore_idx];
 
        read_lock_bh(&idev->lock);
        list_for_each_entry(score->ifa, &idev->addr_list, if_list) {

@@ -1424,6 +1425,7 @@ static void __ipv6_dev_get_saddr(struct net *net,
                                in6_ifa_hold(score->ifa);
 
                                swap(hiscore, score);
+                               hiscore_idx = 1 - hiscore_idx;
 
                                /* restore our iterator */
                                score->ifa = hiscore->ifa;

@@ -1434,18 +1436,20 @@ static void __ipv6_dev_get_saddr(struct net *net,
        }
 out:
        read_unlock_bh(&idev->lock);
+       return hiscore_idx;
 }
 
 int ipv6_dev_get_saddr(struct net *net, const struct net_device *dst_dev,
                       const struct in6_addr *daddr, unsigned int prefs,
                       struct in6_addr *saddr)
 {
-       struct ipv6_saddr_score scores[2], *hiscore = &scores[1];
+       struct ipv6_saddr_score scores[2], *hiscore;
        struct ipv6_saddr_dst dst;
        struct inet6_dev *idev;
        struct net_device *dev;
        int dst_type;
        bool use_oif_addr = false;
+       int hiscore_idx = 0;
 
        dst_type = __ipv6_addr_type(daddr);
        dst.addr = daddr;

@@ -1454,8 +1458,8 @@ int ipv6_dev_get_saddr(struct net *net, const struct net_device *dst_dev,
        dst.label = ipv6_addr_label(net, daddr, dst_type, dst.ifindex);
        dst.prefs = prefs;
 
-       hiscore->rule = -1;
-       hiscore->ifa = NULL;
+       scores[hiscore_idx].rule = -1;
+       scores[hiscore_idx].ifa = NULL;
 
        rcu_read_lock();
 

@@ -1480,17 +1484,19 @@ int ipv6_dev_get_saddr(struct net *net, const struct net_device *dst_dev,
        }
 
        if (use_oif_addr) {
-               __ipv6_dev_get_saddr(net, &dst, prefs, saddr, idev, scores);
+               if (idev)
+                       hiscore_idx = __ipv6_dev_get_saddr(net, &dst, prefs, saddr, idev, scores, hiscore_idx);
        } else {
                for_each_netdev_rcu(net, dev) {
                        idev = __in6_dev_get(dev);
                        if (!idev)
                                continue;
-                       __ipv6_dev_get_saddr(net, &dst, prefs, saddr, idev, scores);
+                       hiscore_idx = __ipv6_dev_get_saddr(net, &dst, prefs, saddr, idev, scores, hiscore_idx);
                }
        }
        rcu_read_unlock();
 
+       hiscore = &scores[hiscore_idx];
        if (!hiscore->ifa)
                return -EADDRNOTAVAIL;
 

Quoted 7 lines

Quoted 7 lines

--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
吉藤英明 <[email protected]>
ミラクル・リナックス株式会社 技術本部 サポート部

I am testing this patch which may be a little simpler. Also idev needs to be checked after __in6_dev_get

Commit 9131f3de2 ("ipv6: Do not iterate over all interfaces when finding source address on specific interface.") did not properly update best source address available. Plus, it introduced possible NULL pointer dereference.

Bug was reported by Erik Kline <[email protected]>.
Based on patch proposed by Hajime Tazaki <[email protected]>.

Fixes 9131f3de24db4dc12199aede7d931e6703e97f3b ("ipv6: Do not

iterate over all interfaces when finding source address on specific interface.")

Signed-off-by YOSHIFUJI Hideaki <[email protected]>

---
 net/ipv6/addrconf.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

net/ipv6/addrconf.c +18 -13

index 4ab74d5..4c9a024 100644
@@ -1358,14 +1358,15 @@ out:
        return ret;
 }
 
-static void __ipv6_dev_get_saddr(struct net *net,
-                                struct ipv6_saddr_dst *dst,
-                                unsigned int prefs,
-                                const struct in6_addr *saddr,
-                                struct inet6_dev *idev,
-                                struct ipv6_saddr_score *scores)
+static int __ipv6_dev_get_saddr(struct net *net,
+                               struct ipv6_saddr_dst *dst,
+                               unsigned int prefs,
+                               const struct in6_addr *saddr,
+                               struct inet6_dev *idev,
+                               struct ipv6_saddr_score *scores,
+                               int hiscore_idx)
 {
-       struct ipv6_saddr_score *score = &scores[0], *hiscore = &scores[1];
+       struct ipv6_saddr_score *score = &scores[1 - hiscore_idx], *hiscore = &scores[hiscore_idx];
 
        read_lock_bh(&idev->lock);
        list_for_each_entry(score->ifa, &idev->addr_list, if_list) {

@@ -1424,6 +1425,7 @@ static void __ipv6_dev_get_saddr(struct net *net,
                                in6_ifa_hold(score->ifa);
 
                                swap(hiscore, score);
+                               hiscore_idx = 1 - hiscore_idx;
 
                                /* restore our iterator */
                                score->ifa = hiscore->ifa;

@@ -1434,18 +1436,20 @@ static void __ipv6_dev_get_saddr(struct net *net,
        }
 out:
        read_unlock_bh(&idev->lock);
+       return hiscore_idx;
 }
 
 int ipv6_dev_get_saddr(struct net *net, const struct net_device *dst_dev,
                       const struct in6_addr *daddr, unsigned int prefs,
                       struct in6_addr *saddr)
 {
-       struct ipv6_saddr_score scores[2], *hiscore = &scores[1];
+       struct ipv6_saddr_score scores[2], *hiscore;
        struct ipv6_saddr_dst dst;
        struct inet6_dev *idev;
        struct net_device *dev;
        int dst_type;
        bool use_oif_addr = false;
+       int hiscore_idx = 0;
 
        dst_type = __ipv6_addr_type(daddr);
        dst.addr = daddr;

@@ -1454,8 +1458,8 @@ int ipv6_dev_get_saddr(struct net *net, const struct net_device *dst_dev,
        dst.label = ipv6_addr_label(net, daddr, dst_type, dst.ifindex);
        dst.prefs = prefs;
 
-       hiscore->rule = -1;
-       hiscore->ifa = NULL;
+       scores[hiscore_idx].rule = -1;
+       scores[hiscore_idx].ifa = NULL;
 
        rcu_read_lock();
 

@@ -1480,17 +1484,19 @@ int ipv6_dev_get_saddr(struct net *net, const struct net_device *dst_dev,
        }
 
        if (use_oif_addr) {
-               __ipv6_dev_get_saddr(net, &dst, prefs, saddr, idev, scores);
+               if (idev)
+                       hiscore_idx = __ipv6_dev_get_saddr(net, &dst, prefs, saddr, idev, scores, hiscore_idx);
        } else {
                for_each_netdev_rcu(net, dev) {
                        idev = __in6_dev_get(dev);
                        if (!idev)
                                continue;
-                       __ipv6_dev_get_saddr(net, &dst, prefs, saddr, idev, scores);
+                       hiscore_idx = __ipv6_dev_get_saddr(net, &dst, prefs, saddr, idev, scores, hiscore_idx);
                }
        }
        rcu_read_unlock();
 
+       hiscore = &scores[hiscore_idx];
        if (!hiscore->ifa)
                return -EADDRNOTAVAIL;
 
--

1.9.1

All my tests pass with this applied to net-next/master.

Many thanks!

Acked-by Erik Kline <[email protected]>

Quoted 109 lines · YOSHIFUJI Hideaki/吉藤英明

Commit 9131f3de2 ("ipv6: Do not iterate over all interfaces when finding source address on specific interface.") did not properly update best source address available. Plus, it introduced possible NULL pointer dereference.

Bug was reported by Erik Kline <[email protected]>.
Based on patch proposed by Hajime Tazaki <[email protected]>.

Fixes 9131f3de24db4dc12199aede7d931e6703e97f3b ("ipv6: Do not

iterate over all interfaces when finding source address on specific interface.")

Signed-off-by YOSHIFUJI Hideaki <[email protected]>

---
 net/ipv6/addrconf.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

net/ipv6/addrconf.c +18 -13

index 4ab74d5..4c9a024 100644
@@ -1358,14 +1358,15 @@ out:
        return ret;
 }
 
-static void __ipv6_dev_get_saddr(struct net *net,
-                                struct ipv6_saddr_dst *dst,
-                                unsigned int prefs,
-                                const struct in6_addr *saddr,
-                                struct inet6_dev *idev,
-                                struct ipv6_saddr_score *scores)
+static int __ipv6_dev_get_saddr(struct net *net,
+                               struct ipv6_saddr_dst *dst,
+                               unsigned int prefs,
+                               const struct in6_addr *saddr,
+                               struct inet6_dev *idev,
+                               struct ipv6_saddr_score *scores,
+                               int hiscore_idx)
 {
-       struct ipv6_saddr_score *score = &scores[0], *hiscore = &scores[1];
+       struct ipv6_saddr_score *score = &scores[1 - hiscore_idx], *hiscore = &scores[hiscore_idx];
 
        read_lock_bh(&idev->lock);
        list_for_each_entry(score->ifa, &idev->addr_list, if_list) {

@@ -1424,6 +1425,7 @@ static void __ipv6_dev_get_saddr(struct net *net,
                                in6_ifa_hold(score->ifa);
 
                                swap(hiscore, score);
+                               hiscore_idx = 1 - hiscore_idx;
 
                                /* restore our iterator */
                                score->ifa = hiscore->ifa;

@@ -1434,18 +1436,20 @@ static void __ipv6_dev_get_saddr(struct net *net,
        }
 out:
        read_unlock_bh(&idev->lock);
+       return hiscore_idx;
 }
 
 int ipv6_dev_get_saddr(struct net *net, const struct net_device *dst_dev,
                       const struct in6_addr *daddr, unsigned int prefs,
                       struct in6_addr *saddr)
 {
-       struct ipv6_saddr_score scores[2], *hiscore = &scores[1];
+       struct ipv6_saddr_score scores[2], *hiscore;
        struct ipv6_saddr_dst dst;
        struct inet6_dev *idev;
        struct net_device *dev;
        int dst_type;
        bool use_oif_addr = false;
+       int hiscore_idx = 0;
 
        dst_type = __ipv6_addr_type(daddr);
        dst.addr = daddr;

@@ -1454,8 +1458,8 @@ int ipv6_dev_get_saddr(struct net *net, const struct net_device *dst_dev,
        dst.label = ipv6_addr_label(net, daddr, dst_type, dst.ifindex);
        dst.prefs = prefs;
 
-       hiscore->rule = -1;
-       hiscore->ifa = NULL;
+       scores[hiscore_idx].rule = -1;
+       scores[hiscore_idx].ifa = NULL;
 
        rcu_read_lock();
 

@@ -1480,17 +1484,19 @@ int ipv6_dev_get_saddr(struct net *net, const struct net_device *dst_dev,
        }
 
        if (use_oif_addr) {
-               __ipv6_dev_get_saddr(net, &dst, prefs, saddr, idev, scores);
+               if (idev)
+                       hiscore_idx = __ipv6_dev_get_saddr(net, &dst, prefs, saddr, idev, scores, hiscore_idx);
        } else {
                for_each_netdev_rcu(net, dev) {
                        idev = __in6_dev_get(dev);
                        if (!idev)
                                continue;
-                       __ipv6_dev_get_saddr(net, &dst, prefs, saddr, idev, scores);
+                       hiscore_idx = __ipv6_dev_get_saddr(net, &dst, prefs, saddr, idev, scores, hiscore_idx);
                }
        }
        rcu_read_unlock();
 
+       hiscore = &scores[hiscore_idx];
        if (!hiscore->ifa)
                return -EADDRNOTAVAIL;
 
--

1.9.1

Commit 9131f3de2 ("ipv6: Do not iterate over all interfaces when finding source address on specific interface.") did not properly update best source address available. Plus, it introduced possible NULL pointer dereference.

Bug was reported by Erik Kline <[email protected]>.
Based on patch proposed by Hajime Tazaki <[email protected]>.

Fixes 9131f3de24db4dc12199aede7d931e6703e97f3b ("ipv6: Do not

iterate over all interfaces when finding source address on specific interface.")

Signed-off-by YOSHIFUJI Hideaki <[email protected]>