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

Add Access Control List to MPU ports #765

Merged
merged 22 commits into from
Sep 18, 2023

Conversation

kar-rahul-aws
Copy link
Member

@kar-rahul-aws kar-rahul-aws commented Aug 28, 2023

Description

This PR adds Access Control to kernel objects on a per task basis to MPU ports. The following needs to be defined in the FreeRTOSConfig.h to enable this feature:

#define configUSE_MPU_WRAPPERS_V1 0
#define configENABLE_ACCESS_CONTROL_LIST 1

This PR adds the following new APIs:

void vGrantAccessToTask( TaskHandle_t xTask, TaskHandle_t xTaskToGrantAccess );
void vRevokeAccessToTask( TaskHandle_t xTask, TaskHandle_t xTaskToRevokeAccess );

void vGrantAccessToSemaphore( TaskHandle_t xTask, SemaphoreHandle_t xSemaphoreToGrantAccess );
void vRevokeAccessToSemaphore( TaskHandle_t xTask, SemaphoreHandle_t xSemaphoreToRevokeAccess );

void vGrantAccessToQueue( TaskHandle_t xTask, QueueHandle_t xQueueToGrantAccess );
void vRevokeAccessToQueue( TaskHandle_t xTask, QueueHandle_t xQueueToRevokeAccess );

void vGrantAccessToQueueSet( TaskHandle_t xTask, QueueSetHandle_t xQueueSetToGrantAccess );
void vRevokeAccessToQueueSet( TaskHandle_t xTask, QueueSetHandle_t xQueueSetToRevokeAccess );

void vGrantAccessToEventGroup( TaskHandle_t xTask, EventGroupHandle_t xEventGroupToGrantAccess );
void vRevokeAccessToEventGroup( TaskHandle_t xTask, EventGroupHandle_t xEventGroupToRevokeAccess );

void vGrantAccessToStreamBuffer( TaskHandle_t xTask, StreamBufferHandle_t xStreamBufferToGrantAccess );
void vRevokeAccessToStreamBuffer( TaskHandle_t xTask, StreamBufferHandle_t xStreamBufferToRevokeAccess );

void vGrantAccessToMessageBuffer( TaskHandle_t xTask, MessageBufferHandle_t xMessageBufferToGrantAccess );
void vRevokeAccessToMessageBuffer( TaskHandle_t xTask, MessageBufferHandle_t xMessageBufferToRevokeAccess );

void vGrantAccessToTimer( TaskHandle_t xTask, TimerHandle_t xTimerToGrantAccess );
void vRevokeAccessToTimer( TaskHandle_t xTask, TimerHandle_t xTimerToRevokeAccess );

An unprivileged task by default has access to itself only and no other kernel object. The application writer needs to explicitly grant an unprivileged task access to all the kernel objects it needs. The best place to do that is before starting the scheduler when all the kernel objects are created.

For example, let's say an unprivileged tasks needs access to a queue and an event group, the application writer needs to do the following:

vGrantAccessToQueue( xUnprivilegedTaskHandle, xQueue );
vGrantAccessToEventGroup( xUnprivilegedTaskHandle, xEventGroup );

The application writer MUST revoke all the accesses before deleting a task. Failing to do so will result in undefined behavior. In the above example, the application writer needs to make the following 2 calls before deleting the task:

vRevokeAccessToQueue( xUnprivilegedTaskHandle, xQueue );
vRevokeAccessToEventGroup( xUnprivilegedTaskHandle, xEventGroup );

Test Steps

Tested on the following platforms -

  1. STM32NUCLEO-H7 (Cortex-M7).
  2. nrf9160 (Cortex-M33)
  3. STM32NUCLEOL152RE (Cortex-M3)

Checklist:

  • I have tested my changes. No regression in existing tests.
  • I have modified and/or added unit-tests to cover the code changes in this Pull Request.

Related Issue

This PR addresses the threat “Unprivileged task accessing valid kernel objects using legitimate system calls” in the FreeRTOS Kernel Threat Model -https://www.freertos.org/security/kernel-threat-model.html.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@kar-rahul-aws kar-rahul-aws requested a review from a team as a code owner August 28, 2023 15:21
@kar-rahul-aws kar-rahul-aws marked this pull request as draft August 28, 2023 15:21
@codecov
Copy link

codecov bot commented Aug 29, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (7db0e87) 94.35% compared to head (736f4e2) 94.35%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #765   +/-   ##
=======================================
  Coverage   94.35%   94.35%           
=======================================
  Files           6        6           
  Lines        2443     2443           
  Branches      598      598           
=======================================
  Hits         2305     2305           
  Misses         85       85           
  Partials       53       53           
Flag Coverage Δ
unittests 94.35% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@aggarg aggarg marked this pull request as ready for review September 17, 2023 19:02
aggarg
aggarg previously approved these changes Sep 17, 2023
@aggarg aggarg changed the title Access Control List for MPU wrappers Add Access Control List to MPU ports Sep 18, 2023
Copy link
Member

@chinglee-iot chinglee-iot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In mpu_wrappers_v2.c xPortIsAuthorizedToAccessKernelObject is called even when configENABLE_ACCESS_CONTROL_LIST is set to 0. The function call and the if statement can't be optimized by compiler when configENABLE_ACCESS_CONTROL_LIST is set to 0.

    xCallingTaskIsAuthorizedToAccessTask = xPortIsAuthorizedToAccessKernelObject( CONVERT_TO_INTERNAL_INDEX( lIndex ) );

    if( xCallingTaskIsAuthorizedToAccessTask == pdTRUE )
    {
        xInternalTaskHandle = MPU_GetTaskHandleAtIndex( CONVERT_TO_INTERNAL_INDEX( lIndex ) );

        if( xInternalTaskHandle != NULL )
        {
            xReturn = xTaskAbortDelay( xInternalTaskHandle );
        }
    }

Suggest we consider to optimize it in future PR or make access control list required in mpu wrapper V2.

@aggarg
Copy link
Member

aggarg commented Sep 18, 2023

The reason we did not do that is to avoid making the code complex to read. xPortIsAuthorizedToAccessKernelObject returns pdTRUE if ACL is not enabled.

@aggarg aggarg merged commit 170a291 into FreeRTOS:main Sep 18, 2023
15 checks passed
@aggarg aggarg deleted the mpu_wrapper_access_control_list branch September 18, 2023 10:04
laroche pushed a commit to laroche/FreeRTOS-Kernel that referenced this pull request Apr 18, 2024
* Add register test tasks to the CORTEX_MPS2_QEMU_IAR GCC and IAR builds.

* Update header comment in the two added files.

* Fix header checks

* Fix build issue

* update IAR version

---------

Co-authored-by: none <>
Co-authored-by: Rahul Kar <[email protected]>
Co-authored-by: Rahul Kar <[email protected]>
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

Successfully merging this pull request may close these issues.

3 participants