* get_proximity() crashes on interfaces with no addresses
@ 2013-05-12 15:48 Doug Nazar
2013-05-12 18:26 ` Leonardo Chiquitto
0 siblings, 1 reply; 6+ messages in thread
From: Doug Nazar @ 2013-05-12 15:48 UTC (permalink / raw)
To: autofs
[-- Attachment #1: Type: text/plain, Size: 249 bytes --]
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.
Should it also check for the IFF_UP flag?
Doug
[-- Attachment #2: autofs-5.0.7-fix-interface-address-null-check.patch --]
[-- Type: text/plain, Size: 667 bytes --]
diff -ur autofs-5.0.7.orig/modules/replicated.c autofs-5.0.7/modules/replicated.c
--- autofs-5.0.7.orig/modules/replicated.c 2013-05-10 04:48:32.000000000 -0400
+++ autofs-5.0.7/modules/replicated.c 2013-05-10 04:50:07.000000000 -0400
@@ -166,7 +166,7 @@
this = ifa;
while (this) {
if (this->ifa_flags & IFF_POINTOPOINT ||
- this->ifa_addr->sa_data == NULL) {
+ this->ifa_addr == NULL) {
this = this->ifa_next;
continue;
}
@@ -203,7 +203,7 @@
this = ifa;
while (this) {
if (this->ifa_flags & IFF_POINTOPOINT ||
- this->ifa_addr->sa_data == NULL) {
+ this->ifa_addr == NULL) {
this = this->ifa_next;
continue;
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: get_proximity() crashes on interfaces with no addresses
2013-05-12 15:48 get_proximity() crashes on interfaces with no addresses Doug Nazar
@ 2013-05-12 18:26 ` Leonardo Chiquitto
2013-05-12 20:31 ` Doug Nazar
0 siblings, 1 reply; 6+ messages in thread
From: Leonardo Chiquitto @ 2013-05-12 18:26 UTC (permalink / raw)
To: Doug Nazar; +Cc: autofs
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.
Hi,
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:
if (this->ifa_flags & IFF_POINTOPOINT ||
this->ifa_addr == NULL ||
this->ifa_addr->sa_data == NULL) {
this = this->ifa_next;
continue;
}
> Should it also check for the IFF_UP flag?
I think it makes sense to test it.
Thanks,
Leonardo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: get_proximity() crashes on interfaces with no addresses
2013-05-12 18:26 ` Leonardo Chiquitto
@ 2013-05-12 20:31 ` Doug Nazar
2013-05-14 4:13 ` Ian Kent
0 siblings, 1 reply; 6+ messages in thread
From: Doug Nazar @ 2013-05-12 20:31 UTC (permalink / raw)
To: Leonardo Chiquitto; +Cc: autofs
[-- 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;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: get_proximity() crashes on interfaces with no addresses
2013-05-12 20:31 ` Doug Nazar
@ 2013-05-14 4:13 ` Ian Kent
2013-05-14 9:15 ` Doug Nazar
0 siblings, 1 reply; 6+ messages in thread
From: Ian Kent @ 2013-05-14 4:13 UTC (permalink / raw)
To: Doug Nazar; +Cc: Leonardo Chiquitto, autofs
On Sun, 2013-05-12 at 16:31 -0400, Doug Nazar wrote:
> 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.
You would think so, I'll go with this.
>
> >> Should it also check for the IFF_UP flag?
> > I think it makes sense to test it.
Indeed that's probably good to do too.
I'd be interested to hear if this patch functions OK with the IFF_UP
check in place.
In the mean time I'll add the patch to my queue for the next commit.
Ian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: get_proximity() crashes on interfaces with no addresses
2013-05-14 4:13 ` Ian Kent
@ 2013-05-14 9:15 ` Doug Nazar
2013-05-16 0:45 ` Ian Kent
0 siblings, 1 reply; 6+ messages in thread
From: Doug Nazar @ 2013-05-14 9:15 UTC (permalink / raw)
To: Ian Kent; +Cc: Leonardo Chiquitto, autofs
On 5/14/2013 12:13 AM, Ian Kent wrote:
> Indeed that's probably good to do too. I'd be interested to hear if
> this patch functions OK with the IFF_UP check in place. In the mean
> time I'll add the patch to my queue for the next commit.
I've been running it for a day on a few boxes with no problems. The only
additional line is a logging line I had when originally testing.
May 14 04:58:47 cockatrice automount[365]: get_proximity:168:
getifaddrs: ifa_name lo, ifa_flags 0x00010049
May 14 04:58:47 cockatrice automount[365]: get_proximity:168:
getifaddrs: ifa_name eql, ifa_flags 0x00000400
May 14 04:58:47 cockatrice automount[365]: get_proximity:168:
getifaddrs: ifa_name eth0, ifa_flags 0x00011043
May 14 04:58:47 cockatrice automount[365]: get_proximity:168:
getifaddrs: ifa_name wlan0, ifa_flags 0x00001002
May 14 04:58:47 cockatrice automount[365]: get_proximity:168:
getifaddrs: ifa_name wmon0, ifa_flags 0x00011043
IFF_UP is 0x01 so that looks correct. eql was the one that it was
crashing on with no ifa_addr.
Doug
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: get_proximity() crashes on interfaces with no addresses
2013-05-14 9:15 ` Doug Nazar
@ 2013-05-16 0:45 ` Ian Kent
0 siblings, 0 replies; 6+ messages in thread
From: Ian Kent @ 2013-05-16 0:45 UTC (permalink / raw)
To: Doug Nazar; +Cc: Leonardo Chiquitto, autofs
On Tue, 2013-05-14 at 05:15 -0400, Doug Nazar wrote:
> On 5/14/2013 12:13 AM, Ian Kent wrote:
> > Indeed that's probably good to do too. I'd be interested to hear if
> > this patch functions OK with the IFF_UP check in place. In the mean
> > time I'll add the patch to my queue for the next commit.
>
> I've been running it for a day on a few boxes with no problems. The only
> additional line is a logging line I had when originally testing.
>
> May 14 04:58:47 cockatrice automount[365]: get_proximity:168:
> getifaddrs: ifa_name lo, ifa_flags 0x00010049
> May 14 04:58:47 cockatrice automount[365]: get_proximity:168:
> getifaddrs: ifa_name eql, ifa_flags 0x00000400
> May 14 04:58:47 cockatrice automount[365]: get_proximity:168:
> getifaddrs: ifa_name eth0, ifa_flags 0x00011043
> May 14 04:58:47 cockatrice automount[365]: get_proximity:168:
> getifaddrs: ifa_name wlan0, ifa_flags 0x00001002
> May 14 04:58:47 cockatrice automount[365]: get_proximity:168:
> getifaddrs: ifa_name wmon0, ifa_flags 0x00011043
>
> IFF_UP is 0x01 so that looks correct. eql was the one that it was
> crashing on with no ifa_addr.
OK, thanks for this.
>
> Doug
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-05-16 0:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-12 15:48 get_proximity() crashes on interfaces with no addresses Doug Nazar
2013-05-12 18:26 ` Leonardo Chiquitto
2013-05-12 20:31 ` Doug Nazar
2013-05-14 4:13 ` Ian Kent
2013-05-14 9:15 ` Doug Nazar
2013-05-16 0:45 ` Ian Kent
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.