Fixed buffer underflow in prvSelectHighestPriorityTask.

[tobireinhard]

Fixed buffer underflow in prvSelectHighestPriorityTask.

Description

The function prvSelectHighestPriorityTask is called from vTaskSwitchContext whenever the scheduler selects a new task to run. It can also be called from vTaskSuspend before the scheduler is running and before the idle tasks have been created.

In the latter case, the global variable uxTopReadyPriority is decreased to -1. During the next regular context switch, prvSelectHighestPriorityTask tries to access the global ready list array pxReadyTasksLists[ uxCurrentPriority ]. This causes a memory error.

Test Steps

Initialize the ready lists stored in pxReadyTasksLists to be empty and call prvSelectHighestPriorityTask. Check that after the the function terminates, the global variable uxTopReadyPriority has value -1. Call vTaskSwitchContext and check that it causes a buffer underflow.

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

[johnrhen]

Thanks for submitting this, we'll take a look

[kstribrnAmzn]

Our CI is a little bit of a pain right now. I'm going to push a commit to your change to update this line. This should stop the header check from failing. Essentially Python 3.7.10 isn't available on Ubuntu 20.04 anymore.

This won't have any impact on the functionality of your PR and is purely to fix the CI.

[kstribrnAmzn]

Nit pick, non-blocking: I realize 0 isn't exactly a magic number in this context BUT I would prefer this line to say uxTopReadyPriority > tskIDLE_PRIORITY. This would read like "The Top Ready Priority cannot be less than the idle task" rather than "The Top Ready Priority cannot be less than 0". While these are functionally the same, pinning this to the idle task would make it clearer that no task can be lower in priority than the idle task.

[AniruddhaKanhere]

The formatting should be something like the following (to adhere to the Kernel's formatting style)

Suggested change

	if(uxTopReadyPriority > 0) {
	uxTopReadyPriority--;
	}
	/* The top ready priority cannot be less than the idle task's priority. */
	if( uxTopReadyPriority > tskIDLE_PRIORITY )
	{
	uxTopReadyPriority--;
	}

[AniruddhaKanhere]

Hello @tobireinhard,

Just a couple of comments regarding formatting (also included the suggestion by @kstribrnAmzn). If they seem correct to you, would you mind pushing them?

Thanks,
Aniruddha

[AniruddhaKanhere]

Similarly here as well

Suggested change

	if(uxCurrentPriority > 0) {
	uxCurrentPriority--;
	}
	/* The top ready priority cannot be less than the idle task's priority. */
	if( uxCurrentPriority > tskIDLE_PRIORITY )
	{
	uxCurrentPriority--;
	}

[chinglee-iot]

It can also be called from vTaskSuspend before the scheduler is running

I would like to share some observation about this problem.
Every core starts with an idle task before scheduler started in SMP implementation ( reference prvAddNewTaskToReadyList ).
prvSelectHighestPriorityTask won't be called in vTaskSuspend before the scheduler started due to xTaskRunState can only be taskTASK_NOT_RUNNING for non-idle task ( Reference prvInitialiseNewTask and vTaskSuspend ) and idle tasks are created in xTaskStartScheduler.

Suggest we consider to update the vTaskSuspend function and ensure that prvSelectHighestPriorityTask won't be called before scheduler started in another PR.

[aggarg]

Closing this PR as it is no longer needed after #610.