Possible bug retrieving IfIndex in newlib - winsup/cygwin/net.cc

Possible bug retrieving IfIndex in newlib - winsup/cygwin/net.cc

[David Bean]

Good Day,

I have been working on porting Samba 4.11 to Cygwin for a few days and ran into an odd issue. Samba configures its interfaces in several steps, but it relies pretty heavily on getting information from the interface structures populated by Cygwin. While I was walking through the call map I noticed a lot of the data is populated by get_ifs and that routine may have a bug in its handling of IfIndex. Basically it seems to copy the IPv4 index at all times even though it may need Ipv6Index.

the same code exists at line 1904 and 1808 as well. I believe the current code may fail to retrieve the correct adapter index if IPv6 is enabled on the interface.
this is current code:  ifp->ifa_hwdata.ifa_ifindex = pap->IfIndex;

I think it should be:  ifp->ifa_hwdata.ifa_ifindex = (sa->sa_family == AF_INET
                                         ? pap->IfIndex : pap->Ipv6Index);

This is the model used by the lines retrieving the other elements with dual locations on the windows side.

I am unable to debug this to confirm it, but please let me know if this is actually a bug or not.

Sincerely,

David Bean



--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: Possible bug retrieving IfIndex in newlib - winsup/cygwin/net.cc

[Corinna Vinschen]

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

Hi David,

On Oct 22 15:56, David Bean wrote:
> Good Day,
> 
> I have been working on porting Samba 4.11 to Cygwin for a few days and ran into an odd issue. Samba configures its interfaces in several steps, but it relies pretty heavily on getting information from the interface structures populated by Cygwin. While I was walking through the call map I noticed a lot of the data is populated by get_ifs and that routine may have a bug in its handling of IfIndex. Basically it seems to copy the IPv4 index at all times even though it may need Ipv6Index.
> 
> the same code exists at line 1904 and 1808 as well. I believe the current code may fail to retrieve the correct adapter index if IPv6 is enabled on the interface.
> this is current code:  ifp->ifa_hwdata.ifa_ifindex = pap->IfIndex;
> 
> I think it should be:  ifp->ifa_hwdata.ifa_ifindex = (sa->sa_family == AF_INET
>                                          ? pap->IfIndex : pap->Ipv6Index);
> 
> This is the model used by the lines retrieving the other elements with dual locations on the windows side.
> 
> I am unable to debug this to confirm it, but please let me know if this is actually a bug or not.

The code in line 1808 only handles AF_INET anyway, so it shouldn't
use IpV6IfIndex.  As for line 1904, I didn't perform this distinction
because in all my testing the indices were the same.

Do you have proof that this isn't always the case?  If so, I check in
a patch.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: Possible bug retrieving IfIndex in newlib - winsup/cygwin/net.cc

[David Bean]

Hello Corrina,

No I do not, it just stuck out while I was looking through the code hunting down another problem. I have had a long history of bumping into undocumented issues with Windows over the years and have become somewhat untrusting with the windows API. I was looking through net.cc because a samba call to if_nametoindex was failing to return an index, but it seems that Cygwin just passes that call through to the IP Help API.

Have a good one,

David Bean

________________________________
From: Corinna Vinschen <corinna-cygwin@cygwin.com>
Sent: Tuesday, October 22, 2019 2:39 PM
To: David Bean <d_bean@hotmail.com>
Cc: cygwin@cygwin.com <cygwin@cygwin.com>
Subject: Re: Possible bug retrieving IfIndex in newlib - winsup/cygwin/net.cc

Hi David,

On Oct 22 15:56, David Bean wrote:
> Good Day,
>
> I have been working on porting Samba 4.11 to Cygwin for a few days and ran into an odd issue. Samba configures its interfaces in several steps, but it relies pretty heavily on getting information from the interface structures populated by Cygwin. While I was walking through the call map I noticed a lot of the data is populated by get_ifs and that routine may have a bug in its handling of IfIndex. Basically it seems to copy the IPv4 index at all times even though it may need Ipv6Index.
>
> the same code exists at line 1904 and 1808 as well. I believe the current code may fail to retrieve the correct adapter index if IPv6 is enabled on the interface.
> this is current code:  ifp->ifa_hwdata.ifa_ifindex = pap->IfIndex;
>
> I think it should be:  ifp->ifa_hwdata.ifa_ifindex = (sa->sa_family == AF_INET
>                                          ? pap->IfIndex : pap->Ipv6Index);
>
> This is the model used by the lines retrieving the other elements with dual locations on the windows side.
>
> I am unable to debug this to confirm it, but please let me know if this is actually a bug or not.

The code in line 1808 only handles AF_INET anyway, so it shouldn't
use IpV6IfIndex.  As for line 1904, I didn't perform this distinction
because in all my testing the indices were the same.

Do you have proof that this isn't always the case?  If so, I check in
a patch.


Thanks,
Corinna

--
Corinna Vinschen
Cygwin Maintainer

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: Possible bug retrieving IfIndex in newlib - winsup/cygwin/net.cc

[Corinna Vinschen]

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

Hi David,

On Oct 22 18:19, David Bean wrote:
> Hello Corrina,

s/rrin/rinn/ ;)

>> From: Corinna Vinschen
>> Hi David,
>> 
>> On Oct 22 15:56, David Bean wrote:
>> > Good Day,
>> >
>> > I have been working on porting Samba 4.11 to Cygwin for a few days
>> > and ran into an odd issue. Samba configures its interfaces in
>> > several steps, but it relies pretty heavily on getting information
>> > from the interface structures populated by Cygwin. While I was
>> > walking through the call map I noticed a lot of the data is
>> > populated by get_ifs and that routine may have a bug in its
>> > handling of IfIndex. Basically it seems to copy the IPv4 index at
>> > all times even though it may need Ipv6Index.
>> >
>> > the same code exists at line 1904 and 1808 as well. I believe the
>> > current code may fail to retrieve the correct adapter index if IPv6
>> > is enabled on the interface.
>> > this is current code:  ifp->ifa_hwdata.ifa_ifindex = pap->IfIndex;
>> >
>> > I think it should be:  ifp->ifa_hwdata.ifa_ifindex = (sa->sa_family == AF_INET
>> >                                          ? pap->IfIndex : pap->Ipv6Index);
>> >
>> > This is the model used by the lines retrieving the other elements
>> > with dual locations on the windows side.
>> >
>> > I am unable to debug this to confirm it, but please let me know if
>> > this is actually a bug or not.
>> 
>> The code in line 1808 only handles AF_INET anyway, so it shouldn't
>> use IpV6IfIndex.  As for line 1904, I didn't perform this distinction
>> because in all my testing the indices were the same.
>> 
>> Do you have proof that this isn't always the case?  If so, I check in
>> a patch.
>
> No I do not, it just stuck out while I was looking through the code
> hunting down another problem. I have had a long history of bumping
> into undocumented issues with Windows over the years and have become
> somewhat untrusting with the windows API. I was looking through net.cc
> because a samba call to if_nametoindex was failing to return an index,
> but it seems that Cygwin just passes that call through to the IP Help
> API.

I was just about to apply the patch nevertheless, when I remembered
another reason not to use IpV6IfIndex.  *Iff* IpV6IfIndex is different
from the default IfIndex, what does it keep from colliding with another,
already used IfIndex of another interface?  That would lead to two
interfaces having the same index in Cygwin.

> Have a good one,

Same to you,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]