Re: get_proximity() crashes on interfaces with no addresses

[Doug Nazar <nazard@nazar.ca>]

[-- Attachment #1: Type: text/plain, Size: 768 bytes --]

On 5/12/2013 2:26 PM, Leonardo Chiquitto wrote:
> On Sun, May 12, 2013 at 5:48 PM, Doug Nazar <nazard@nazar.ca> wrote:
>> Since commit aa6f7793 [autofs-5.0.7 - fix ipv6 proximity calculation] uses
>> getifaddrs however it crashes on interfaces with no addresses. Fix the NULL
>> check to ignore interfaces with no addresses.
> Your patch removes the check on ifa_addr->sa_data. I'm wondering if
> it's possible to have a valid ifa_addr and a NULL sa_data. Do you know?
> Maybe it's safer to just test both:

I don't see how. ifa_addr is of type struct sockaddr. The sa_data field 
is a char array and only has meaning after interpreting the sa_family field.

>> Should it also check for the IFF_UP flag?
> I think it makes sense to test it.
>

Ok, here you go.

Doug


[-- Attachment #2: autofs-5.0.7-fix-interface-address-null-check2.patch --]
[-- Type: text/plain, Size: 1279 bytes --]

commit d0b5d4961004a41f6881b2ac2bb32ba3002654e5
Author: Doug Nazar <nazard@nazar.ca>
Date:   Sun May 12 16:22:04 2013 -0400

    Since commit aa6f7793 [autofs-5.0.7 - fix ipv6 proximity calculation]
    get_proximity() uses getifaddrs however it crashes on interfaces with no
    addresses. Fix the NULL check to ignore interfaces with no addresses.
    
    Also skip interfaces which are not currently running.

diff --git a/modules/replicated.c b/modules/replicated.c
index 26f64b8..6dbdade 100644
--- a/modules/replicated.c
+++ b/modules/replicated.c
@@ -165,8 +165,9 @@ static unsigned int get_proximity(struct sockaddr *host_addr)
 
 	this = ifa;
 	while (this) {
-		if (this->ifa_flags & IFF_POINTOPOINT ||
-		    this->ifa_addr->sa_data == NULL) {
+		if (!(this->ifa_flags & IFF_UP) ||
+		    this->ifa_flags & IFF_POINTOPOINT ||
+		    this->ifa_addr == NULL) {
 			this = this->ifa_next;
 			continue;
 		}
@@ -202,8 +203,9 @@ static unsigned int get_proximity(struct sockaddr *host_addr)
 
 	this = ifa;
 	while (this) {
-		if (this->ifa_flags & IFF_POINTOPOINT ||
-		    this->ifa_addr->sa_data == NULL) {
+		if (!(this->ifa_flags & IFF_UP) ||
+		    this->ifa_flags & IFF_POINTOPOINT ||
+		    this->ifa_addr == NULL) {
 			this = this->ifa_next;
 			continue;
 		}