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

ostimer.c functions not using semaphore #92

Closed
skliper opened this issue Sep 30, 2019 · 7 comments
Closed

ostimer.c functions not using semaphore #92

skliper opened this issue Sep 30, 2019 · 7 comments

Comments

@skliper
Copy link
Contributor

skliper commented Sep 30, 2019

The VxWorks ostimer.c has a static table and a semaphore to protect it, as in:
{{{
static OS_timer_record_t OS_timer_table[OS_MAX_TIMERS];
static uint32 os_clock_accuracy;

/*
** The Semaphore for protecting the above table
*/
static SEM_ID OS_timer_table_sem;
}}}

Unfortunately, only OS_TimerCreate(), and OS_TimerGetInfo() use that semaphore.

'''OS_TimerCreate(), OS_TimerSet(), OS_TimerDelete() do not''' and they '''modify''' the table. '''OS_TimerGetIdByName()''' and the internal '''OS_TimerSignalHandler()''' also access (read) the table without using the semaphore.

The OS_TimerAPIInit() creates the semaphore, the time conversion functions don't access the table.

Recommend fixing all the functions that access or modify the table to acquire the semaphore after passing input argument checks.

@skliper skliper self-assigned this Sep 30, 2019
@skliper skliper added the bug label Sep 30, 2019
@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Imported from trac issue 69. Created by abrown4 on 2015-07-01T16:43:49, last modified: 2015-12-01T14:13:03

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by abrown4 on 2015-07-08 13:11:06:

commit: [changeset:1a6e450] Trac #92, Fixed VxWorks ostimer.c to properly use semaphore.
Built upon ostimer.c coverage tests on Track #45. Made unit test changes to differentiate between failures due to Trac #92 and Trac #88.

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by abrown4 on 2015-07-08 13:22:36:

Opened Trac #94 for the posix version, which has similar problems.
Not all methods in the RTEMS version are protected... that may need its own issue as well. Created #95.

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by abrown4 on 2015-07-28 12:36:33:

Reviewed the above commit with #103 in mind.

If the functions that access the table, OS_TimerGetIdByName() and the internal OS_TimerSignalHandler(), follow the same paradigm then they don't need modification. Similarly, OS_TimerSet() modifies the table entry but the access pattern might not collide with other tasks given the expected usage patterns. Discuss.

MUST CHANGE: OS_TimerSignalHandler() invokes a callback. This should not occur while the table is locked.

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by abrown4 on 2015-07-28 14:58:16:

commit: [changeset:270c2dc] Trac #92, Removed table locks in ostimer.c OS_TimerSignalHandler().
Locking a global table while calling a non-osal callback isn't a good idea.

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by abrown4 on 2015-08-20 18:24:31:

From the above commits, as of #45 [changeset:6a9db70], the white-box unit test still shows that OS_TimerSignalHandler() doesn't safely access the OS_timer_table (3 test failures from ostimer_wb test).

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by abrown4 on 2015-11-25 12:24:13:

Traceability: The vxworks code changes from #92, [changeset:1a6e450] and [changeset:270c2dc] have been reflected in #138 [changeset:ab2d6d7]. Recommend closing #92 in favor of #138.

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

No branches or pull requests

1 participant