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

NETLINK_ROUTE socket binding not available on Android 11+, can we live without it for Android? #6251

Closed
gubatron opened this issue Jun 7, 2021 · 11 comments

Comments

@gubatron
Copy link
Contributor

gubatron commented Jun 7, 2021

libtorrent version (or branch): 1.2.x

platform/architecture: android

Once again Android is about to introduce mandatory breaking changes, all apps must be fixed before November 2021.

image

Some users already running Android 11 are reporting crashes with jlibtorrent

02-24 16:24:34.925 E/mac-restrictions(1901): idm.internet.download.manager.plus:DownloadService tried to call bind() on a NETLINK_ROUTE socket, which is not allowed. Please follow instructions at go/netlink-bug if this app behaves incorrectly.
02-24 16:24:35.030 E/IorapForwardingService(1456): No service published for: iorapd
02-24 16:24:35.030 E/IorapForwardingService(1456): android.os.ServiceManager$ServiceNotFoundException: No service published for: iorapd

Luckily I see that in:
include/libtorrent/netlink.hpp
src/enum_net.cpp
src/ip_notifier.cpp

all the code with NETLINK_ROUTE exists within a compile time defined (I think) TORRENT_USE_NETLINK flag.

I have 2 questions:

  • Is the TORRENT_USE_NETLINK a default compilation flag set for android and linux? or is it optional and it has to be set explicitly (which would mean those crashes they're reporting are not coming from jlibtorrent since I can't find a mention to the flag in any of my build scripts)
  • What's the benefit of using NETLINK_ROUTE sockets, will I have to drop some features in Linux and Android if this flag is disabled when building libtorrent?
@ssiloti
Copy link
Collaborator

ssiloti commented Jun 8, 2021

Netlink is enabled on all Linux systems, including Android. Netlink is used to enumerate network interfaces and routes as well as get notification when the system's network interfaces change. Unfortunately it looks like Google intends to remove the ability of native code to do any of those things.

Without the ability to enumerate network interfaces libtorrent will have to fall back to listening on the unspecified address. This will produce incorrect behavior if the system is multi-homed (e.g. has both Wi-Fi and LTE connections). NAT-PMP support will not work without the ability to enumerate routes. Setting outgoing_interfaces won't work. There may be other issues as well, I don't think the cannot enumerate case is well tested.

@gubatron
Copy link
Contributor Author

gubatron commented Jun 9, 2021

Netlink is used to enumerate network interfaces and routes as well as get notification when the system's network interfaces change.

As of API 24, according to the stackoverflow thread shared by @wrtorrent, the solution is to enumerate interfaces using ifaddrs. Not sure if it ifaddrs also covers routes and network interface change callbacks.

Nonetheless, I see the libtorrent code uses ifaddr when not using netlink

@gubatron
Copy link
Contributor Author

gubatron commented Jun 26, 2021

@ssiloti, @arvidn
Sorry to bother you, but I'm stuck now for 2 days
How can I force the compilation to not use NETLINK and instead use IFADDRS sockets?

I'm trying this on my config.jam (perhaps redundantly)
-DTORRENT_USE_IFADDRS=1
-DTORRENT_USE_NETLINK=0

import os ;

ANDROID_TOOLCHAIN = [ os.environ ANDROID_TOOLCHAIN ] ;
ANDROID_API = [ os.environ android_api ] ;

using clang : x86 : $(ANDROID_TOOLCHAIN)/bin/i686-linux-android$(ANDROID_API)-clang++ :
    <cxxflags>-fPIC
    <cxxflags>-std=c++14
    <cxxflags>-DANDROID
    <cxxflags>-DTORRENT_USE_IFADDRS=1
    <cxxflags>-DTORRENT_USE_NETLINK=0
    <cxxflags>-D__STDC_FORMAT_MACROS
    <cxxflags>-D__USE_FILE_OFFSET64
    <cxxflags>-D_FILE_OFFSET_BITS=64
    <cxxflags>-DWITH_IPP=OFF
    <cxxflags>-frtti
    <cxxflags>-fno-strict-aliasing
    <cxxflags>-fvisibility=hidden
    # x86 devices have stack alignment issues, http://b.android.com/222239
    <cxxflags>-mstackrealign
    # debug information
    <cxxflags>-g
    <cxxflags>-gdwarf-4
    <cxxflags>-fdebug-macro
    <cxxflags>-ggdb
    <linkflags>-static-libstdc++
    ;

However when it's building I see a bunch of pre-processors warnings that these macros keep getting redefined, so I think it's not having the effect I wish, which is to disable NETLINK sockets and use instead IFADDR (given the SELinux limitations imposed on Android 11+)

In file included from /src/libtorrent/src/kademlia/sample_infohashes.cpp:33:
In file included from /src/libtorrent/include/libtorrent/kademlia/sample_infohashes.hpp:38:
In file included from /src/libtorrent/include/libtorrent/kademlia/traversal_algorithm.hpp:40:
In file included from /src/libtorrent/include/libtorrent/fwd.hpp:36:
/src/libtorrent/include/libtorrent/config.hpp:174:9: warning: 'TORRENT_USE_IFADDRS' macro redefined [-Wmacro-redefined]
#define TORRENT_USE_IFADDRS 0
        ^
<command line>:2:9: note: previous definition is here
#define TORRENT_USE_IFADDRS 1
        ^
2 warnings generated.
clang-linux.compile.c++.without-pch bin/release/android/x86/src/kademlia/traversal_algorithm.o
In file included from /src/libtorrent/src/kademlia/traversal_algorithm.cpp:33:
In file included from /src/libtorrent/include/libtorrent/kademlia/traversal_algorithm.hpp:40:
In file included from /src/libtorrent/include/libtorrent/fwd.hpp:36:
/src/libtorrent/include/libtorrent/config.hpp:173:9: warning: 'TORRENT_USE_NETLINK' macro redefined [-Wmacro-redefined]
#define TORRENT_USE_NETLINK 1
        ^
<command line>:3:9: note: previous definition is here
#define TORRENT_USE_NETLINK 0

Once the library is used on Android's SDK 30 (Android 11) it's forbidden by SELinux to bind netlink sockets.
I keep getting this error
onListenFailed(): error_message=listening on 0.0.0.0:0 (device: ) failed: [enum_if] [TCP] Permission denied

I wish I could build for Android 29 and be done, but as of November 2021 it will be mandatory making this upgrade and a bunch of torrent clients using jlibtorrent will break or won't be able to support android 11 devices, which we see are getting a very fast uptake the last few months.

@aldenml
Copy link
Contributor

aldenml commented Jun 26, 2021

@gubatron , the ability to switch to ifaddrs for android requires a patch around the related logic in config.h, but that's not a big issue. The problem I see is that getifaddrs,freeifaddrs are only available for API >= 24. If you follow the path of just selecting the logic statically (at compile time) you will need to drop support for quite a few users. The other more complex solution is to detect and select at runtime.

You will definitely lose the ability to get notifications on network interfaces changes, but we were already prepared for this with session_handle::reopen_network_sockets :)

@aldenml
Copy link
Contributor

aldenml commented Jun 26, 2021

@arvidn, I looked at this in detail, and there is no way enum_routes can handle android API >= 30 with the current code. Do you think reopen_network_sockets can receive a list of interfaces and routes? any idea of how to able to plug JNI android specific code without ugly hacks?

@gubatron
Copy link
Contributor Author

gubatron commented Jun 26, 2021

@alden

The problem I see is that getifaddrs,freeifaddrs are only available for API >= 24.

This is correct, So far I was able to rebuild no problem with NDK API=24 (which I believe is Android 7.0 Nougat), I rather drop support for Android 6 (not that many users now, I believe it's from 2016) when I look at the existing ratio of Android 10, and increasingly fast growth of Android 11 users.

Screen Shot 2021-06-26 at 1 02 40 PM

But still, I don't think the way I've set the flags has worked.
I've also tried setting, just -DTORRENT_USE_IFADDRS by itself.
Need to try -DTORRENT_USE_NETLINK=0 by itself.

but we were already prepared for this with session_handle::reopen_network_sockets
Good to know.

At this point these are decisions for the app to survive going forward, Google has made some very tough calls on Android 11.

@gubatron
Copy link
Contributor Author

gubatron commented Jun 27, 2021

the ability to switch to ifaddrs for android requires a patch around the related logic in config.h

🙏 Thanks for this @aldenml .

I'll move away from defining those preprocessor constants in the cxxflags in config.jam and instead will experiment with a patch on include/libtorrent/config.hpp (if this is what you mean by "related logic in config.h")

Maybe something like this

diff --git a/include/libtorrent/config.hpp b/include/libtorrent/config.hpp
index 04c32ad68..600e4188d 100644
--- a/include/libtorrent/config.hpp
+++ b/include/libtorrent/config.hpp
@@ -159,6 +159,11 @@ see LICENSE file.
 #define TORRENT_HAS_FTELLO 0
 #endif // API < 24
 
+#if __ANDROID_API__ >= 24
+#define TORRENT_USE_IFADDRS 1
+#define TORRENT_USE_NETLINK 0
+#endif
+
 #else // ANDROID
 
 // posix_fallocate() is not available in glibc under these condition

@gubatron
Copy link
Contributor Author

gubatron commented Aug 12, 2021

Managed to create a patch that hardcodes TORRENT_USE_IFADDRS 1 and TORRENT_USE_NETLINK 0 in config.hpp and enum_net.cpp, a libtorrent session can be started successfully from Android 7.0 (SDK 24) and all the way up to Android 11.0 (SDK 30), still have not been able to test on Android 11.

Here's the current patch in case anybody needs it. (to be applied on libtorrent's RC_1_2 branch)

diff --git a/include/libtorrent/alert.hpp b/include/libtorrent/alert.hpp
index ab874a247..9ace38065 100644
--- a/include/libtorrent/alert.hpp
+++ b/include/libtorrent/alert.hpp
@@ -177,7 +177,7 @@ namespace alert_category {
 	// interpreted as -1. For instance, boost.python
 	// does that and fails when assigning it to an
 	// unsigned parameter.
-	constexpr alert_category_t all = alert_category_t::all();
+	 //deleted temporarily because it is defined twice
 
 } // namespace alert_category
 
diff --git a/include/libtorrent/config.hpp b/include/libtorrent/config.hpp
index bec1b4756..e11d7df9d 100644
--- a/include/libtorrent/config.hpp
+++ b/include/libtorrent/config.hpp
@@ -170,8 +170,8 @@ POSSIBILITY OF SUCH DAMAGE.
 
 #define TORRENT_HAS_SYMLINK 1
 #define TORRENT_HAVE_MMAP 1
-#define TORRENT_USE_NETLINK 1
-#define TORRENT_USE_IFADDRS 0
+#define TORRENT_USE_NETLINK 0
+#define TORRENT_USE_IFADDRS 1
 #define TORRENT_USE_IFCONF 1
 #define TORRENT_HAS_SALEN 0
 #define TORRENT_USE_FDATASYNC 1
@@ -185,6 +185,10 @@ POSSIBILITY OF SUCH DAMAGE.
 #define TORRENT_ANDROID
 #define TORRENT_HAS_FALLOCATE 0
 #define TORRENT_USE_ICONV 0
+#undef TORRENT_USE_NETLINK
+#undef TORRENT_USE_IFADDRS
+#define TORRENT_USE_NETLINK 0
+#define TORRENT_USE_IFADDRS 1
 #else // ANDROID
 
 // posix_fallocate() is not available in glibc under these condition
@@ -434,7 +438,7 @@ POSSIBILITY OF SUCH DAMAGE.
 #endif
 
 #ifndef TORRENT_USE_IFADDRS
-#define TORRENT_USE_IFADDRS 0
+#define TORRENT_USE_IFADDRS 1
 #endif
 
 // if preadv() exists, we assume pwritev() does as well
diff --git a/src/enum_net.cpp b/src/enum_net.cpp
index 1ba578c6e..b6b685a5c 100644
--- a/src/enum_net.cpp
+++ b/src/enum_net.cpp
@@ -31,7 +31,10 @@ POSSIBILITY OF SUCH DAMAGE.
 */
 
 #include "libtorrent/config.hpp"
-
+#undef TORRENT_USE_NETLINK
+#undef TORRENT_USE_IFADDRS
+#define TORRENT_USE_IFADDRS 1
+#define TORRENT_USE_NETLINK 0
 #include "libtorrent/enum_net.hpp"
 #include "libtorrent/broadcast_socket.hpp"
 #include "libtorrent/assert.hpp"
@@ -1409,7 +1412,7 @@ int _System __libsocket_sysctl(int* mib, u_int namelen, void *oldp, size_t *oldl
 		}
 
 #else
-#error "don't know how to enumerate network routes on this platform"
+		//#error "don't know how to enumerate network routes on this platform"
 #endif
 		return ret;
 	}

Android clients that use jlibtorrent will be able to get a hold of this fix on the 1.2.14.0 release coming out soon, hopefully keeping everything working for Android 11 and beyond.

@arvidn
Copy link
Owner

arvidn commented Aug 13, 2021

did a fix for this land in RC_1_2?

@gubatron
Copy link
Contributor Author

gubatron commented Aug 13, 2021

Not sure I understand the question.

To build jlibtorrent for android, we currently check out RC_1_2 and apply the patch above.

We tried passing <cxxflags>-DTORRENT_USE_IFADDRS=1 or/and <cxxflags>-DTORRENT_USE_NETLINK=0 via config.jam for android builds and it didn't really work.

That patch is applied only right before building for android as a desperate measure in my build script.

We plan on migrating to RC_2_0 as soon as android lets us breath, the file systems changes Android made (being able to use File objects and regular file paths again) I believe will make it easier for us to migrate jlibtorrent to RC_2_0 without breaking android.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants