Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 664d208a authored by Erik Kline's avatar Erik Kline
Browse files

Fixes for tetheroffload crashes 

Now that we can talk to the HALs (with some out of tree CLs and
"setenforce 0"), several crashes were encountered.

Fixes here include:
    - avoid hidl_handle move semantics
    - check HIDL method status return value (isOk())
    - convert Java short port numbers to ints
    - don't pass nulls to HIDL where Strings are required
      (limitations in parceling)

Test: as follows
    - built
    - flashed
    - booted
    - runtest frameworks-net passes
    - "setenforce 0" and start tethering
Bug: 29337859
Bug: 32163131

Merged-In: I5fa930be0c9eda491cf17bd4de9b55ab33672d25
Merged-In: Id1037d22826f4d426bccfa17dce0962c54518d64
Change-Id: I91314440c3a04e5f2502579b5f06dac9f25cf0cd
(cherry picked from commit 1185459c)
parent 8a945baf

services/core/java/com/android/server/connectivity/tethering/OffloadController.java

+2 −1
Original line number Diff line number Diff line
@@ -161,6 +161,7 @@ public class OffloadController { 
            }

        }



        return mHwInterface.setUpstreamParameters(iface, v4addr, v4gateway, v6gateways);

        return mHwInterface.setUpstreamParameters(

                iface, v4addr, v4gateway, (v6gateways.isEmpty() ? null : v6gateways));

    }

}

services/core/java/com/android/server/connectivity/tethering/OffloadHardwareInterface.java

+12 −2
Original line number Diff line number Diff line
@@ -16,6 +16,8 @@ 


package com.android.server.connectivity.tethering;



import static com.android.internal.util.BitUtils.uint16;



import android.hardware.tetheroffload.control.V1_0.IOffloadControl;

import android.hardware.tetheroffload.control.V1_0.ITetheringOffloadCallback;

import android.hardware.tetheroffload.control.V1_0.NatTimeoutUpdate;
@@ -33,6 +35,9 @@ import java.util.ArrayList; 
 */

public class OffloadHardwareInterface {

    private static final String TAG = OffloadHardwareInterface.class.getSimpleName();

    private static final String NO_INTERFACE_NAME = "";

    private static final String NO_IPV4_ADDRESS = "";

    private static final String NO_IPV4_GATEWAY = "";



    private static native boolean configOffload();
@@ -107,6 +112,11 @@ public class OffloadHardwareInterface { 


    public boolean setUpstreamParameters(

            String iface, String v4addr, String v4gateway, ArrayList<String> v6gws) {

        iface = iface != null ? iface : NO_INTERFACE_NAME;

        v4addr = v4addr != null ? v4addr : NO_IPV4_ADDRESS;

        v4gateway = v4gateway != null ? v4gateway : NO_IPV4_GATEWAY;

        v6gws = v6gws != null ? v6gws : new ArrayList<>();



        final CbResults results = new CbResults();

        try {

            mOffloadControl.setUpstreamParameters(
@@ -143,8 +153,8 @@ public class OffloadHardwareInterface { 
            handler.post(() -> {

                    controlCb.onNatTimeoutUpdate(

                        params.proto,

                        params.src.addr, params.src.port,

                        params.dst.addr, params.dst.port);

                        params.src.addr, uint16(params.src.port),

                        params.dst.addr, uint16(params.dst.port));

            });

        }

    }

services/core/jni/com_android_server_connectivity_tethering_OffloadHardwareInterface.cpp

+6 −5
Original line number Diff line number Diff line
@@ -71,7 +71,7 @@ int conntrackSocket(unsigned groups) { 
// auto-close it (otherwise there would be double-close problems).

//

// Rely upon the compiler to eliminate the constexprs used for clarity.

hidl_handle&& handleFromFileDescriptor(base::unique_fd fd) {

hidl_handle handleFromFileDescriptor(base::unique_fd fd) {

    hidl_handle h;



    NATIVE_HANDLE_DECLARE_STORAGE(storage, 0, 0);
@@ -83,7 +83,7 @@ hidl_handle&& handleFromFileDescriptor(base::unique_fd fd) { 
    static constexpr bool kTakeOwnership = true;

    h.setTo(nh, kTakeOwnership);



    return std::move(h);

    return h;

}



}  // namespace
@@ -116,13 +116,14 @@ static jboolean android_server_connectivity_tethering_OffloadHardwareInterface_c 


    bool rval;

    hidl_string msg;

    configInterface->setHandles(h1, h2,

    const auto status = configInterface->setHandles(h1, h2,

            [&rval, &msg](bool success, const hidl_string& errMsg) {

                rval = success;

                msg = errMsg;

            });

    if (!rval) {

        ALOGE("IOffloadConfig::setHandles() error: %s", msg.c_str());

    if (!status.isOk() || !rval) {

        ALOGE("IOffloadConfig::setHandles() error: '%s' / '%s'",

              status.description().c_str(), msg.c_str());

    }



    return rval;

tests/net/java/com/android/server/connectivity/tethering/OffloadControllerTest.java

+3 −6
Original line number Diff line number Diff line
@@ -155,8 +155,7 @@ public class OffloadControllerTest { 
        lp.setInterfaceName(testIfName);

        offload.setUpstreamLinkProperties(lp);

        inOrder.verify(mHardware, times(1)).setUpstreamParameters(

                eq(testIfName), eq(null), eq(null), mStringArrayCaptor.capture());

        assertTrue(mStringArrayCaptor.getValue().isEmpty());

                eq(testIfName), eq(null), eq(null), eq(null));

        inOrder.verifyNoMoreInteractions();



        final String ipv4Addr = "192.0.2.5";
@@ -164,16 +163,14 @@ public class OffloadControllerTest { 
        lp.addLinkAddress(new LinkAddress(linkAddr));

        offload.setUpstreamLinkProperties(lp);

        inOrder.verify(mHardware, times(1)).setUpstreamParameters(

                eq(testIfName), eq(ipv4Addr), eq(null), mStringArrayCaptor.capture());

        assertTrue(mStringArrayCaptor.getValue().isEmpty());

                eq(testIfName), eq(ipv4Addr), eq(null), eq(null));

        inOrder.verifyNoMoreInteractions();



        final String ipv4Gateway = "192.0.2.1";

        lp.addRoute(new RouteInfo(InetAddress.getByName(ipv4Gateway)));

        offload.setUpstreamLinkProperties(lp);

        inOrder.verify(mHardware, times(1)).setUpstreamParameters(

                eq(testIfName), eq(ipv4Addr), eq(ipv4Gateway), mStringArrayCaptor.capture());

        assertTrue(mStringArrayCaptor.getValue().isEmpty());

                eq(testIfName), eq(ipv4Addr), eq(ipv4Gateway), eq(null));

        inOrder.verifyNoMoreInteractions();



        final String ipv6Gw1 = "fe80::cafe";