Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Android] Fix NetworkInterface.GetAllNetworkInterfaces #76370

Merged

Conversation

simonrozsival
Copy link
Member

@simonrozsival simonrozsival commented Sep 29, 2022

Apparently, since Android API 30, the getifaddrs function returns only AF_INET and AF_INET6 addresses and doesn't return any AF_PACKET. The problem was that we allocated the memoryBlock in such a way that the interfaceList and the addressList overlapped and in the end both contained invalid data. We replaced the default implementation of getifaddrs with one from Xamarin (#71943) based on netlink but that turned out to be an insufficient fix and doesn't work on recent Android versions.

This PR fixes the allocation problem (see pal_interfaceaddresses.c:356-400 and removes the custom getifaddrs implementation in pal_ifaddrs.h and pal_ifaddrs.c.

Reverts #71943 and restores the previous implementation from #62780
Fixes #75809
Fixes #76493
Fixes dotnet/android#6973

Followed up by PR #76541

@ghost
Copy link

ghost commented Sep 29, 2022

Tagging subscribers to 'arch-android': @steveisok, @akoeplinger
See info in area-owners.md if you want to be subscribed.

Issue Details

Apparently, since Android API 30, the getifaddrs function returns only AP_INET and AP_INET6 addresses and doesn't return any AP_PACKET. The problem was that we allocated the memoryBlock in such a way that the interfaceList and the addressList overlapped and in the end both contained invalid data. We replaced the default implementation of getifaddrs with one from Xamarin (#71943) based on netlink but that turned out to be an insufficient fix and doesn't work on recent Android versions.

This PR fixes the allocation problem (see pal_interfaceaddresses.c:356-400 and removes the custom getifaddrs implementation in pal_ifaddrs.h and pal_ifaddrs.c.

Reverts #71943 and restores the previous implementation from #62780
Fixes #75809
Fixes dotnet/android#6973

Author: simonrozsival
Assignees: -
Labels:

os-android

Milestone: -

@ghost ghost assigned simonrozsival Sep 29, 2022
@ghost
Copy link

ghost commented Sep 29, 2022

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

Apparently, since Android API 30, the getifaddrs function returns only AP_INET and AP_INET6 addresses and doesn't return any AP_PACKET. The problem was that we allocated the memoryBlock in such a way that the interfaceList and the addressList overlapped and in the end both contained invalid data. We replaced the default implementation of getifaddrs with one from Xamarin (#71943) based on netlink but that turned out to be an insufficient fix and doesn't work on recent Android versions.

This PR fixes the allocation problem (see pal_interfaceaddresses.c:356-400 and removes the custom getifaddrs implementation in pal_ifaddrs.h and pal_ifaddrs.c.

Reverts #71943 and restores the previous implementation from #62780
Fixes #75809
Fixes dotnet/android#6973

Author: simonrozsival
Assignees: -
Labels:

area-System.Net, os-android

Milestone: -

@simonrozsival
Copy link
Member Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@simonrozsival
Copy link
Member Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

simonrozsival and others added 2 commits September 29, 2022 14:56
Co-authored-by: Alexander Köplinger <alex.koeplinger@outlook.com>
@simonrozsival
Copy link
Member Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@simonrozsival
Copy link
Member Author

Failing tests are unrelated to this PR and many of them are known issues:

@simonrozsival simonrozsival merged commit 81b70b1 into dotnet:main Oct 3, 2022
simonrozsival added a commit to simonrozsival/runtime that referenced this pull request Oct 3, 2022
* Revert "[Android] Port getifaddrs implementation from Xamarin.Android (dotnet#71943)"

This reverts commit 1de4a5c.

* Fix allocating memory block for interfaces and addresses on recent Android SDKs

* Detect loopback interface on Android

* Add comment with explanation

* Simplify the changes to be closer to the original code

* Fix build

* Fix typos

Co-authored-by: Alexander Köplinger <alex.koeplinger@outlook.com>

* Improve comment

* Indent using spaces instead of tabs

* Remove check for ifaddrs.h

* Add ANDROID_GETIFADDRS_WORKAROUND

* Update comment

Co-authored-by: Alexander Köplinger <alex.koeplinger@outlook.com>
carlossanlop pushed a commit that referenced this pull request Oct 5, 2022
…erfaces (#76565)

* [Android] Fix NetworkInterface.GetAllNetworkInterfaces (#76370)

* Revert "[Android] Port getifaddrs implementation from Xamarin.Android (#71943)"

This reverts commit 1de4a5c.

* Fix allocating memory block for interfaces and addresses on recent Android SDKs

* Detect loopback interface on Android

* Add comment with explanation

* Simplify the changes to be closer to the original code

* Fix build

* Fix typos

Co-authored-by: Alexander Köplinger <alex.koeplinger@outlook.com>

* Improve comment

* Indent using spaces instead of tabs

* Remove check for ifaddrs.h

* Add ANDROID_GETIFADDRS_WORKAROUND

* Update comment

Co-authored-by: Alexander Köplinger <alex.koeplinger@outlook.com>

* [Android] Fix NetworkInterface.GetAllNetworkInterfaces on API 21-23 (#76541)

* Bring back pal_ifaddrs

* Update the header file

Co-authored-by: Alexander Köplinger <alex.koeplinger@outlook.com>
@steveisok
Copy link
Member

/backport to release/6.0

@github-actions
Copy link
Contributor

Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/3283870988

@github-actions
Copy link
Contributor

@steveisok backporting to release/6.0 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: Revert "[Android] Port getifaddrs implementation from Xamarin.Android (#71943)"
Using index info to reconstruct a base tree...
M	src/mono/cmake/config.h.in
A	src/native/libs/Common/pal_config.h.in
A	src/native/libs/System.Native/CMakeLists.txt
A	src/native/libs/System.Native/pal_ifaddrs.c
A	src/native/libs/System.Native/pal_ifaddrs.h
A	src/native/libs/System.Native/pal_interfaceaddresses.c
A	src/native/libs/configure.cmake
Falling back to patching base and 3-way merge...
Auto-merging src/mono/cmake/config.h.in
Auto-merging src/libraries/Native/Unix/configure.cmake
Auto-merging src/libraries/Native/Unix/System.Native/pal_interfaceaddresses.c
CONFLICT (rename/delete): src/native/libs/System.Native/pal_ifaddrs.h deleted in Revert "[Android] Port getifaddrs implementation from Xamarin.Android (#71943)" and renamed to src/libraries/Native/Unix/System.Native/pal_ifaddrs.h in HEAD. Version HEAD of src/libraries/Native/Unix/System.Native/pal_ifaddrs.h left in tree.
CONFLICT (rename/delete): src/native/libs/System.Native/pal_ifaddrs.c deleted in Revert "[Android] Port getifaddrs implementation from Xamarin.Android (#71943)" and renamed to src/libraries/Native/Unix/System.Native/pal_ifaddrs.c in HEAD. Version HEAD of src/libraries/Native/Unix/System.Native/pal_ifaddrs.c left in tree.
Auto-merging src/libraries/Native/Unix/System.Native/CMakeLists.txt
Auto-merging src/libraries/Native/Unix/Common/pal_config.h.in
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Revert "[Android] Port getifaddrs implementation from Xamarin.Android (#71943)"
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

@steveisok
Copy link
Member

@simonrozsival When you have a moment, can you please try and backport this to release/6.0?

simonrozsival added a commit to simonrozsival/runtime that referenced this pull request Oct 20, 2022
* Revert "[Android] Port getifaddrs implementation from Xamarin.Android (dotnet#71943)"

This reverts commit 1de4a5c.

* Fix allocating memory block for interfaces and addresses on recent Android SDKs

* Detect loopback interface on Android

* Add comment with explanation

* Simplify the changes to be closer to the original code

* Fix build

* Fix typos

Co-authored-by: Alexander Köplinger <alex.koeplinger@outlook.com>

* Improve comment

* Indent using spaces instead of tabs

* Remove check for ifaddrs.h

* Add ANDROID_GETIFADDRS_WORKAROUND

* Update comment

Co-authored-by: Alexander Köplinger <alex.koeplinger@outlook.com>
carlossanlop pushed a commit that referenced this pull request Nov 3, 2022
…erfaces (#77260)

* [Android] Fix NetworkInterface.GetAllNetworkInterfaces (#76370)

* Revert "[Android] Port getifaddrs implementation from Xamarin.Android (#71943)"

This reverts commit 1de4a5c.

* Fix allocating memory block for interfaces and addresses on recent Android SDKs

* Detect loopback interface on Android

* Add comment with explanation

* Simplify the changes to be closer to the original code

* Fix build

* Fix typos

Co-authored-by: Alexander Köplinger <alex.koeplinger@outlook.com>

* Improve comment

* Indent using spaces instead of tabs

* Remove check for ifaddrs.h

* Add ANDROID_GETIFADDRS_WORKAROUND

* Update comment

Co-authored-by: Alexander Köplinger <alex.koeplinger@outlook.com>

* [Android] Fix NetworkInterface.GetAllNetworkInterfaces on API 21-23 (#76541)

* Bring back pal_ifaddrs

* Update the header file

Co-authored-by: Alexander Köplinger <alex.koeplinger@outlook.com>
@ghost ghost locked as resolved and limited conversation to collaborators Nov 18, 2022
@karelz karelz added this to the 8.0.0 milestone Mar 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.