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

Commit 4f5ede64 authored by Yu Shan's avatar Yu Shan
Browse files

Add ScheduleInfo validity check. 

Define the max task data size. Requires remote access HAL to return
invalid arg if ScheduleInfo is not valid.

Updated the reference impl to add the checks.

Test: atest RemoteAccessServiceUnitTest
Bug: 317405128
Change-Id: Ia17dda2683c3bcc861542cb2fbd812ce8bd368aa
parent d710e2ab

automotive/remoteaccess/aidl_api/android.hardware.automotive.remoteaccess/current/android/hardware/automotive/remoteaccess/ScheduleInfo.aidl

+1 −0
Original line number Diff line number Diff line
@@ -41,4 +41,5 @@ parcelable ScheduleInfo { 
  int count;

  long startTimeInEpochSeconds;

  long periodicInSeconds;

  const int MAX_TASK_DATA_SIZE_IN_BYTES = 10240;

}

automotive/remoteaccess/android/hardware/automotive/remoteaccess/IRemoteAccess.aidl

+3 −0
Original line number Diff line number Diff line
@@ -167,6 +167,9 @@ interface IRemoteAccess { 
     * {@code scheduleId} for this client exists.

     *

     * <p>Must return {@code EX_ILLEGAL_ARGUMENT} if the task type is not supported.

     *

     * <p>Must return {@code EX_ILLEGLA_ARGUMENT} if the scheduleInfo is not valid (e.g. count is

     * a negative number).

     */

    void scheduleTask(in ScheduleInfo scheduleInfo);

automotive/remoteaccess/android/hardware/automotive/remoteaccess/ScheduleInfo.aidl

+3 −0
Original line number Diff line number Diff line
@@ -21,6 +21,7 @@ import android.hardware.automotive.remoteaccess.TaskType; 
@VintfStability

@JavaDerive(equals=true, toString=true)

parcelable ScheduleInfo {

    const int MAX_TASK_DATA_SIZE_IN_BYTES = 10240;

    /**

     * The ID used to identify the client this schedule is for. This must be one of the

     * preconfigured remote access serverless client ID defined in car service resource
@@ -41,6 +42,8 @@ parcelable ScheduleInfo { 
     * executed. It is not interpreted/parsed by the Android system.

     *

     * <p>This is only used for {@code TaskType.CUSTOM}.

     *

     * <p>The data size must be less than {@link MAX_TASK_DATA_SIZE_IN_BYTES}.

     */

    byte[] taskData;

    /**

automotive/remoteaccess/hal/default/src/RemoteAccessService.cpp

+20 −1
Original line number Diff line number Diff line
@@ -331,6 +331,24 @@ ScopedAStatus RemoteAccessService::scheduleTask(const ScheduleInfo& scheduleInfo 
    ClientContext context;

    ScheduleTaskRequest request = {};

    ScheduleTaskResponse response = {};



    if (scheduleInfo.count < 0) {

        return ScopedAStatus::fromExceptionCodeWithMessage(EX_ILLEGAL_ARGUMENT,

                                                           "count must be >= 0");

    }

    if (scheduleInfo.startTimeInEpochSeconds < 0) {

        return ScopedAStatus::fromExceptionCodeWithMessage(EX_ILLEGAL_ARGUMENT,

                                                           "startTimeInEpochSeconds must be >= 0");

    }

    if (scheduleInfo.periodicInSeconds < 0) {

        return ScopedAStatus::fromExceptionCodeWithMessage(EX_ILLEGAL_ARGUMENT,

                                                           "periodicInSeconds must be >= 0");

    }

    if (scheduleInfo.taskData.size() > scheduleInfo.MAX_TASK_DATA_SIZE_IN_BYTES) {

        return ScopedAStatus::fromExceptionCodeWithMessage(EX_ILLEGAL_ARGUMENT,

                                                           "task data too big");

    }



    request.mutable_scheduleinfo()->set_clientid(scheduleInfo.clientId);

    request.mutable_scheduleinfo()->set_scheduleid(scheduleInfo.scheduleId);

    request.mutable_scheduleinfo()->set_data(scheduleInfo.taskData.data(),
@@ -348,7 +366,8 @@ ScopedAStatus RemoteAccessService::scheduleTask(const ScheduleInfo& scheduleInfo 
        case ErrorCode::OK:

            return ScopedAStatus::ok();

        case ErrorCode::INVALID_ARG:

            return ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT);

            return ScopedAStatus::fromExceptionCodeWithMessage(

                    EX_ILLEGAL_ARGUMENT, "received invalid_arg from grpc server");

        default:

            // Should not happen.

            return ScopedAStatus::fromServiceSpecificErrorWithMessage(

automotive/remoteaccess/hal/default/test/RemoteAccessServiceUnitTest.cpp

+65 −1
Original line number Diff line number Diff line
@@ -473,7 +473,71 @@ TEST_F(RemoteAccessServiceUnitTest, TestScheduleTask) { 
    EXPECT_EQ(grpcRequest.scheduleinfo().periodicinseconds(), kTestPeriodicInSeconds);

}



TEST_F(RemoteAccessServiceUnitTest, TestScheduleTask_InvalidArg) {

TEST_F(RemoteAccessServiceUnitTest, TestScheduleTask_InvalidCount) {

    ScheduleInfo scheduleInfo = {

            .clientId = kTestClientId,

            .scheduleId = kTestScheduleId,

            .taskData = kTestData,

            .count = -1,

            .startTimeInEpochSeconds = kTestStartTimeInEpochSeconds,

            .periodicInSeconds = kTestPeriodicInSeconds,

    };



    ScopedAStatus status = getService()->scheduleTask(scheduleInfo);



    ASSERT_FALSE(status.isOk());

    ASSERT_EQ(status.getExceptionCode(), EX_ILLEGAL_ARGUMENT);

}



TEST_F(RemoteAccessServiceUnitTest, TestScheduleTask_InvalidStartTimeInEpochSeconds) {

    ScheduleInfo scheduleInfo = {

            .clientId = kTestClientId,

            .scheduleId = kTestScheduleId,

            .taskData = kTestData,

            .count = kTestCount,

            .startTimeInEpochSeconds = -1,

            .periodicInSeconds = kTestPeriodicInSeconds,

    };



    ScopedAStatus status = getService()->scheduleTask(scheduleInfo);



    ASSERT_FALSE(status.isOk());

    ASSERT_EQ(status.getExceptionCode(), EX_ILLEGAL_ARGUMENT);

}



TEST_F(RemoteAccessServiceUnitTest, TestScheduleTask_InvalidPeriodicInSeconds) {

    ScheduleInfo scheduleInfo = {

            .clientId = kTestClientId,

            .scheduleId = kTestScheduleId,

            .taskData = kTestData,

            .count = kTestCount,

            .startTimeInEpochSeconds = kTestStartTimeInEpochSeconds,

            .periodicInSeconds = -1,

    };



    ScopedAStatus status = getService()->scheduleTask(scheduleInfo);



    ASSERT_FALSE(status.isOk());

    ASSERT_EQ(status.getExceptionCode(), EX_ILLEGAL_ARGUMENT);

}



TEST_F(RemoteAccessServiceUnitTest, TestScheduleTask_TaskDataTooLarge) {

    ScheduleInfo scheduleInfo = {

            .clientId = kTestClientId,

            .scheduleId = kTestScheduleId,

            .taskData = std::vector<uint8_t>(ScheduleInfo::MAX_TASK_DATA_SIZE_IN_BYTES + 1),

            .count = kTestCount,

            .startTimeInEpochSeconds = kTestStartTimeInEpochSeconds,

            .periodicInSeconds = kTestPeriodicInSeconds,

    };



    ScopedAStatus status = getService()->scheduleTask(scheduleInfo);



    ASSERT_FALSE(status.isOk());

    ASSERT_EQ(status.getExceptionCode(), EX_ILLEGAL_ARGUMENT);

}



TEST_F(RemoteAccessServiceUnitTest, TestScheduleTask_InvalidArgFromGrpcServer) {

    EXPECT_CALL(*getGrpcWakeupClientStub(), ScheduleTask)

            .WillOnce([]([[maybe_unused]] ClientContext* context,

                         [[maybe_unused]] const ScheduleTaskRequest& request,