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

vxworks osloader.c functions not using mutex #99

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

vxworks osloader.c functions not using mutex #99

skliper opened this issue Sep 30, 2019 · 6 comments

Comments

@skliper
Copy link
Contributor

skliper commented Sep 30, 2019

The osloader.c functions do not properly protect the OS_module_table, OS_sym_table_file_id, and OS_symbol_table_size with the OS_module_table_mut.

These items are being read from and changed outside of the mutex.

(Discovered as part of #45 coverage testing.)

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Imported from trac issue 76. Created by abrown4 on 2015-07-15T11:47:13, last modified: 2015-12-01T14:13:24

@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

Trac comment by abrown4 on 2015-07-20 15:43:12:

Discovered OS_ModuleLoad() also wasn't always cleaning up when encountering an error and performing an early return with
{{{
OS_module_table[possible_moduleid].free = TRUE ;
}}}
Put in cleanup code and added test checks.

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by abrown4 on 2015-07-20 16:14:32:

commit [changeset:ecfe66e] Trac #99, osloader mutex unit test update.
commit [changeset:be8d33e] Trac #99, osloader mutex fix.

BEFORE THE osloader.c FIX:
Test failures:
FAIL: getNSemTake() > 0, File: osloader_testcase.c, Line: 365
FAIL: getNSemTake() > 0, File: osloader_testcase.c, Line: 459
FAIL: retval != OS_SUCCESS, File: osloader_testcase.c, Line: 603
FAIL: Terminating filename null in table.name, File: osloader_testcase.c, Line: 605
FAIL: getNSemTake() == 0, File: osloader_testcase.c, Line: 609
FAIL: Table is empty, File: osloader_testcase.c, Line: 612
FAIL: Table is empty, File: osloader_testcase.c, Line: 739
FAIL: getNSemTake() > 0, File: osloader_testcase.c, Line: 831
FAIL: getNSemTake() > 0, File: osloader_testcase.c, Line: 876
FAIL: getNSemTake() > 0, File: osloader_testcase.c, Line: 900
FAIL: getNSemTake() > 0, File: osloader_testcase.c, Line: 932
FAIL: getNSemTake() > 0, File: osloader_testcase.c, Line: 1006
FAIL: getNSemTake() > 0, File: osloader_testcase.c, Line: 1032

Tests Executed: 34
Assert Pass Count: 273
Assert Fail Count: 13

AFTER THE FIX:
Tests failing due to #98 (module_name too long)
FAIL: retval != OS_SUCCESS, File: osloader_testcase.c, Line: 603
FAIL: Terminating filename null in table.name, File: osloader_testcase.c, Line: 605
FAIL: getNSemTake() == 0, File: osloader_testcase.c, Line: 609
FAIL: Table is empty, File: osloader_testcase.c, Line: 612

Tests Executed: 34
Assert Pass Count: 282
Assert Fail Count: 4

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by abrown4 on 2015-07-28 15:09:15:

Comment for review: The changes for this issue come down to the premise that it is safer to globally lock the OS_module_table and serialize access during OS_SymbolTableDump(), OS_ModuleLoad(), OS_ModuleUnload(), and OS_ModuleInfo(). If that is not the case then recommend removing these changes and addressing this issue at a later time.

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by abrown4 on 2015-08-20 18:29:17:

From the above commits, as of #45 [changeset:6a9db70], the white-box unit test still shows that OS_SymbolTableDump(), and OS_ModuleInfo() don't safely access the OS_module_table (5 test failures from osloader_wb test).

@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Trac comment by abrown4 on 2015-11-25 12:59:12:

Note, after an 8/19 code inspection (#45), [changeset:ab00bb5] removed proposed locking for OS_SymbolTableDump and [changeset:abdf238] removed proposed locking for OS_ModuleInfo. Note, locking is still not present in those functions.

Traceability: the remaining osloader.c locking changes are reflected in #138, [changeset:ab2d6d7]. Recommend closure of this issue in favor of #138 with the caveat that several osloader.c functions are (still) not thread-safe.

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