Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
3646d59
checkpoint
jsinglet Aug 3, 2022
f73ee76
fix up test case
jsinglet Aug 8, 2022
eca7609
checkpoint
jsinglet Aug 8, 2022
16f46f2
adding usage patterns for C++/C
jsinglet Aug 9, 2022
e82f522
work
jsinglet Aug 9, 2022
7349926
work
jsinglet Aug 10, 2022
b4dd1a2
work
jsinglet Aug 10, 2022
7f3b492
migrate
jsinglet Aug 10, 2022
c69756f
import
jsinglet Aug 10, 2022
5decd4c
new queries
jsinglet Aug 10, 2022
8cfed06
new tests
jsinglet Aug 10, 2022
fec7e5d
work
jsinglet Aug 10, 2022
64c1c9a
tests
jsinglet Aug 10, 2022
aa02fd2
test
jsinglet Aug 10, 2022
a678379
quewry
jsinglet Aug 10, 2022
f4b0b04
Merge remote-tracking branch 'origin/main' into jsinglet/concurrency3
jsinglet Aug 10, 2022
dbd81f2
import help
jsinglet Aug 10, 2022
c1db06c
change note
jsinglet Aug 10, 2022
fc9c206
new ruels
jsinglet Aug 10, 2022
83844f4
adding header
jsinglet Aug 10, 2022
2a42deb
test case
jsinglet Aug 10, 2022
c58e190
docs
jsinglet Aug 10, 2022
0ae0c00
fixup of rules
jsinglet Aug 11, 2022
444bced
moving some classes into the library
jsinglet Aug 11, 2022
9306f0a
rf
jsinglet Aug 11, 2022
27874f0
rf
jsinglet Aug 11, 2022
f5d8a70
import
jsinglet Aug 11, 2022
e5e9965
import
jsinglet Aug 11, 2022
149e546
rf
jsinglet Aug 11, 2022
c295bb2
rf
jsinglet Aug 11, 2022
6241d5a
rf
jsinglet Aug 11, 2022
261fe71
test case
jsinglet Aug 11, 2022
6548c33
test case
jsinglet Aug 11, 2022
7c7434a
Merge branch 'main' into jsinglet/concurrency3
jsinglet Aug 11, 2022
d0b94f9
docs
jsinglet Aug 11, 2022
630ec72
Merge remote-tracking branch 'origin/jsinglet/concurrency3' into jsin…
jsinglet Aug 11, 2022
1a68c18
docs
jsinglet Aug 12, 2022
8357878
docs
jsinglet Aug 12, 2022
27fa710
placeholders
jsinglet Aug 12, 2022
6164b13
test case update
jsinglet Aug 12, 2022
926564c
Update cpp/common/test/rules/donotallowamutextogooutofscopewhilelocke…
Aug 12, 2022
e3a192d
Merge branch 'main' into jsinglet/concurrency3
Aug 12, 2022
2bbf844
scope
jsinglet Aug 12, 2022
5d86cd8
implementation notes
jsinglet Aug 12, 2022
f43662a
Merge remote-tracking branch 'origin/jsinglet/concurrency3' into jsin…
jsinglet Aug 12, 2022
782e57c
revert
jsinglet Aug 12, 2022
e8776e0
revert
jsinglet Aug 12, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
172 changes: 172 additions & 0 deletions c/cert/src/rules/CON31-C/DoNotAllowAMutexToGoOutOfScopeWhileLocked.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
# CON31-C: Do not destroy a mutex while it is locked

This query implements the CERT-C rule CON31-C:

> Do not destroy a mutex while it is locked


## Description

Mutexes are used to protect shared data structures being concurrently accessed. If a mutex is destroyed while a thread is blocked waiting for that mutex, [critical sections](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-criticalsections) and shared data are no longer protected.

The C Standard, 7.26.4.1, paragraph 2 \[[ISO/IEC 9899:2011](https://wiki.sei.cmu.edu/confluence/display/c/AA.+Bibliography#AA.Bibliography-ISO-IEC9899-2011)\], states

> The `mtx_destroy` function releases any resources used by the mutex pointed to by `mtx`. No threads can be blocked waiting for the mutex pointed to by `mtx`.


This statement implies that destroying a mutex while a thread is waiting on it is [undefined behavior](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-undefinedbehavior).

## Noncompliant Code Example

This noncompliant code example creates several threads that each invoke the `do_work()` function, passing a unique number as an ID. The `do_work()` function initializes the `lock` mutex if the argument is 0 and destroys the mutex if the argument is `max_threads - 1`. In all other cases, the `do_work()` function provides normal processing. Each thread, except the final cleanup thread, increments the atomic `completed` variable when it is finished.

Unfortunately, this code contains several race conditions, allowing the mutex to be destroyed before it is unlocked. Additionally, there is no guarantee that `lock` will be initialized before it is passed to `mtx_lock()`. Each of these behaviors is [undefined](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-undefinedbehavior).

```cpp
#include <stdatomic.h>
#include <stddef.h>
#include <threads.h>

mtx_t lock;
/* Atomic so multiple threads can modify safely */
atomic_int completed = ATOMIC_VAR_INIT(0);
enum { max_threads = 5 };

int do_work(void *arg) {
int *i = (int *)arg;

if (*i == 0) { /* Creation thread */
if (thrd_success != mtx_init(&lock, mtx_plain)) {
/* Handle error */
}
atomic_store(&completed, 1);
} else if (*i < max_threads - 1) { /* Worker thread */
if (thrd_success != mtx_lock(&lock)) {
/* Handle error */
}
/* Access data protected by the lock */
atomic_fetch_add(&completed, 1);
if (thrd_success != mtx_unlock(&lock)) {
/* Handle error */
}
} else { /* Destruction thread */
mtx_destroy(&lock);
}
return 0;
}

int main(void) {
thrd_t threads[max_threads];

for (size_t i = 0; i < max_threads; i++) {
if (thrd_success != thrd_create(&threads[i], do_work, &i)) {
/* Handle error */
}
}
for (size_t i = 0; i < max_threads; i++) {
if (thrd_success != thrd_join(threads[i], 0)) {
/* Handle error */
}
}
return 0;
}

```

## Compliant Solution

This compliant solution eliminates the race conditions by initializing the mutex in `main()` before creating the threads and by destroying the mutex in `main()` after joining the threads:

```cpp
#include <stdatomic.h>
#include <stddef.h>
#include <threads.h>

mtx_t lock;
/* Atomic so multiple threads can increment safely */
atomic_int completed = ATOMIC_VAR_INIT(0);
enum { max_threads = 5 };

int do_work(void *dummy) {
if (thrd_success != mtx_lock(&lock)) {
/* Handle error */
}
/* Access data protected by the lock */
atomic_fetch_add(&completed, 1);
if (thrd_success != mtx_unlock(&lock)) {
/* Handle error */
}

return 0;
}

int main(void) {
thrd_t threads[max_threads];

if (thrd_success != mtx_init(&lock, mtx_plain)) {
/* Handle error */
}
for (size_t i = 0; i < max_threads; i++) {
if (thrd_success != thrd_create(&threads[i], do_work, NULL)) {
/* Handle error */
}
}
for (size_t i = 0; i < max_threads; i++) {
if (thrd_success != thrd_join(threads[i], 0)) {
/* Handle error */
}
}

mtx_destroy(&lock);
return 0;
}

```

## Risk Assessment

Destroying a mutex while it is locked may result in invalid control flow and data corruption.

<table> <tbody> <tr> <th> Rule </th> <th> Severity </th> <th> Likelihood </th> <th> Remediation Cost </th> <th> Priority </th> <th> Level </th> </tr> <tr> <td> CON31-C </td> <td> Medium </td> <td> Probable </td> <td> High </td> <td> <strong>P4</strong> </td> <td> <strong>L3</strong> </td> </tr> </tbody> </table>


## Automated Detection

<table> <tbody> <tr> <th> Tool </th> <th> Version </th> <th> Checker </th> <th> Description </th> </tr> <tr> <td> <a> Astrée </a> </td> <td> 22.04 </td> <td> </td> <td> Supported, but no explicit checker </td> </tr> <tr> <td> <a> CodeSonar </a> </td> <td> 7.0p0 </td> <td> <strong>CONCURRENCY.LOCALARG</strong> </td> <td> Local Variable Passed to Thread </td> </tr> <tr> <td> <a> Helix QAC </a> </td> <td> 2022.2 </td> <td> <strong>C4961, C4962</strong> </td> <td> </td> </tr> <tr> <td> <a> PRQA QA-C </a> </td> <td> 9.7 </td> <td> <strong>4961, 4962 </strong> </td> <td> </td> </tr> <tr> <td> <a> Parasoft C/C++test </a> </td> <td> 2022.1 </td> <td> <strong>CERT_C-CON31-a</strong> <strong>CERT_C-CON31-b</strong> <strong>CERT_C-CON31-c</strong> </td> <td> Do not destroy another thread's mutex Do not use resources that have been freed Do not free resources using invalid pointers </td> </tr> <tr> <td> <a> Polyspace Bug Finder </a> </td> <td> R2022a </td> <td> <a> CERT C: Rule CON31-C </a> </td> <td> Checks for destruction of locked mutex (rule fully covered) </td> </tr> </tbody> </table>


## Related Vulnerabilities

Search for [vulnerabilities](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-vulnerability) resulting from the violation of this rule on the [CERT website](https://www.kb.cert.org/vulnotes/bymetric?searchview&query=FIELD+KEYWORDS+contains+CON31-C).

## Related Guidelines

[Key here](https://wiki.sei.cmu.edu/confluence/display/c/How+this+Coding+Standard+is+Organized#HowthisCodingStandardisOrganized-RelatedGuidelines) (explains table format and definitions)

<table> <tbody> <tr> <th> Taxonomy </th> <th> Taxonomy item </th> <th> Relationship </th> </tr> <tr> <td> <a> CWE 2.11 </a> </td> <td> <a> CWE-667 </a> , Improper Locking </td> <td> 2017-07-10: CERT: Rule subset of CWE </td> </tr> </tbody> </table>


## CERT-CWE Mapping Notes

[Key here](https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=87152408#HowthisCodingStandardisOrganized-CERT-CWEMappingNotes) for mapping notes

**CWE-667 and CON31-C/POS48-C**

Intersection( CON31-C, POS48-C) = Ø

CWE-667 = Union, CON31-C, POS48-C, list) where list =

* Locking &amp; Unlocking issues besides unlocking another thread’s C mutex or pthread mutex.

## Bibliography

<table> <tbody> <tr> <td> \[ <a> ISO/IEC 9899:2011 </a> \] </td> <td> 7.26.4.1, "The <code>mtx_destroy</code> Function" </td> </tr> </tbody> </table>


## Implementation notes

None

## References

* CERT-C: [CON31-C: Do not destroy a mutex while it is locked](https://wiki.sei.cmu.edu/confluence/display/c)
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/**
* @id c/cert/do-not-allow-a-mutex-to-go-out-of-scope-while-locked
* @name CON31-C: Do not destroy a mutex while it is locked
* @description Allowing a mutex to go out of scope while it is locked removes protections around
* shared resources.
* @kind problem
* @precision high
* @problem.severity error
* @tags external/cert/id/con31-c
* correctness
* concurrency
* external/cert/obligation/rule
*/

import cpp
import codingstandards.c.cert
import codingstandards.cpp.rules.donotallowamutextogooutofscopewhilelocked.DoNotAllowAMutexToGoOutOfScopeWhileLocked

class DoNotAllowAMutexToGoOutOfScopeWhileLockedQuery extends DoNotAllowAMutexToGoOutOfScopeWhileLockedSharedQuery {
DoNotAllowAMutexToGoOutOfScopeWhileLockedQuery() {
this = Concurrency3Package::doNotAllowAMutexToGoOutOfScopeWhileLockedQuery()
}
}
172 changes: 172 additions & 0 deletions c/cert/src/rules/CON31-C/DoNotDestroyAMutexWhileItIsLocked.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
# CON31-C: Do not destroy a mutex while it is locked

This query implements the CERT-C rule CON31-C:

> Do not destroy a mutex while it is locked


## Description

Mutexes are used to protect shared data structures being concurrently accessed. If a mutex is destroyed while a thread is blocked waiting for that mutex, [critical sections](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-criticalsections) and shared data are no longer protected.

The C Standard, 7.26.4.1, paragraph 2 \[[ISO/IEC 9899:2011](https://wiki.sei.cmu.edu/confluence/display/c/AA.+Bibliography#AA.Bibliography-ISO-IEC9899-2011)\], states

> The `mtx_destroy` function releases any resources used by the mutex pointed to by `mtx`. No threads can be blocked waiting for the mutex pointed to by `mtx`.


This statement implies that destroying a mutex while a thread is waiting on it is [undefined behavior](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-undefinedbehavior).

## Noncompliant Code Example

This noncompliant code example creates several threads that each invoke the `do_work()` function, passing a unique number as an ID. The `do_work()` function initializes the `lock` mutex if the argument is 0 and destroys the mutex if the argument is `max_threads - 1`. In all other cases, the `do_work()` function provides normal processing. Each thread, except the final cleanup thread, increments the atomic `completed` variable when it is finished.

Unfortunately, this code contains several race conditions, allowing the mutex to be destroyed before it is unlocked. Additionally, there is no guarantee that `lock` will be initialized before it is passed to `mtx_lock()`. Each of these behaviors is [undefined](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-undefinedbehavior).

```cpp
#include <stdatomic.h>
#include <stddef.h>
#include <threads.h>

mtx_t lock;
/* Atomic so multiple threads can modify safely */
atomic_int completed = ATOMIC_VAR_INIT(0);
enum { max_threads = 5 };

int do_work(void *arg) {
int *i = (int *)arg;

if (*i == 0) { /* Creation thread */
if (thrd_success != mtx_init(&lock, mtx_plain)) {
/* Handle error */
}
atomic_store(&completed, 1);
} else if (*i < max_threads - 1) { /* Worker thread */
if (thrd_success != mtx_lock(&lock)) {
/* Handle error */
}
/* Access data protected by the lock */
atomic_fetch_add(&completed, 1);
if (thrd_success != mtx_unlock(&lock)) {
/* Handle error */
}
} else { /* Destruction thread */
mtx_destroy(&lock);
}
return 0;
}

int main(void) {
thrd_t threads[max_threads];

for (size_t i = 0; i < max_threads; i++) {
if (thrd_success != thrd_create(&threads[i], do_work, &i)) {
/* Handle error */
}
}
for (size_t i = 0; i < max_threads; i++) {
if (thrd_success != thrd_join(threads[i], 0)) {
/* Handle error */
}
}
return 0;
}

```

## Compliant Solution

This compliant solution eliminates the race conditions by initializing the mutex in `main()` before creating the threads and by destroying the mutex in `main()` after joining the threads:

```cpp
#include <stdatomic.h>
#include <stddef.h>
#include <threads.h>

mtx_t lock;
/* Atomic so multiple threads can increment safely */
atomic_int completed = ATOMIC_VAR_INIT(0);
enum { max_threads = 5 };

int do_work(void *dummy) {
if (thrd_success != mtx_lock(&lock)) {
/* Handle error */
}
/* Access data protected by the lock */
atomic_fetch_add(&completed, 1);
if (thrd_success != mtx_unlock(&lock)) {
/* Handle error */
}

return 0;
}

int main(void) {
thrd_t threads[max_threads];

if (thrd_success != mtx_init(&lock, mtx_plain)) {
/* Handle error */
}
for (size_t i = 0; i < max_threads; i++) {
if (thrd_success != thrd_create(&threads[i], do_work, NULL)) {
/* Handle error */
}
}
for (size_t i = 0; i < max_threads; i++) {
if (thrd_success != thrd_join(threads[i], 0)) {
/* Handle error */
}
}

mtx_destroy(&lock);
return 0;
}

```

## Risk Assessment

Destroying a mutex while it is locked may result in invalid control flow and data corruption.

<table> <tbody> <tr> <th> Rule </th> <th> Severity </th> <th> Likelihood </th> <th> Remediation Cost </th> <th> Priority </th> <th> Level </th> </tr> <tr> <td> CON31-C </td> <td> Medium </td> <td> Probable </td> <td> High </td> <td> <strong>P4</strong> </td> <td> <strong>L3</strong> </td> </tr> </tbody> </table>


## Automated Detection

<table> <tbody> <tr> <th> Tool </th> <th> Version </th> <th> Checker </th> <th> Description </th> </tr> <tr> <td> <a> Astrée </a> </td> <td> 22.04 </td> <td> </td> <td> Supported, but no explicit checker </td> </tr> <tr> <td> <a> CodeSonar </a> </td> <td> 7.0p0 </td> <td> <strong>CONCURRENCY.LOCALARG</strong> </td> <td> Local Variable Passed to Thread </td> </tr> <tr> <td> <a> Helix QAC </a> </td> <td> 2022.2 </td> <td> <strong>C4961, C4962</strong> </td> <td> </td> </tr> <tr> <td> <a> PRQA QA-C </a> </td> <td> 9.7 </td> <td> <strong>4961, 4962 </strong> </td> <td> </td> </tr> <tr> <td> <a> Parasoft C/C++test </a> </td> <td> 2022.1 </td> <td> <strong>CERT_C-CON31-a</strong> <strong>CERT_C-CON31-b</strong> <strong>CERT_C-CON31-c</strong> </td> <td> Do not destroy another thread's mutex Do not use resources that have been freed Do not free resources using invalid pointers </td> </tr> <tr> <td> <a> Polyspace Bug Finder </a> </td> <td> R2022a </td> <td> <a> CERT C: Rule CON31-C </a> </td> <td> Checks for destruction of locked mutex (rule fully covered) </td> </tr> </tbody> </table>


## Related Vulnerabilities

Search for [vulnerabilities](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-vulnerability) resulting from the violation of this rule on the [CERT website](https://www.kb.cert.org/vulnotes/bymetric?searchview&query=FIELD+KEYWORDS+contains+CON31-C).

## Related Guidelines

[Key here](https://wiki.sei.cmu.edu/confluence/display/c/How+this+Coding+Standard+is+Organized#HowthisCodingStandardisOrganized-RelatedGuidelines) (explains table format and definitions)

<table> <tbody> <tr> <th> Taxonomy </th> <th> Taxonomy item </th> <th> Relationship </th> </tr> <tr> <td> <a> CWE 2.11 </a> </td> <td> <a> CWE-667 </a> , Improper Locking </td> <td> 2017-07-10: CERT: Rule subset of CWE </td> </tr> </tbody> </table>


## CERT-CWE Mapping Notes

[Key here](https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=87152408#HowthisCodingStandardisOrganized-CERT-CWEMappingNotes) for mapping notes

**CWE-667 and CON31-C/POS48-C**

Intersection( CON31-C, POS48-C) = Ø

CWE-667 = Union, CON31-C, POS48-C, list) where list =

* Locking &amp; Unlocking issues besides unlocking another thread’s C mutex or pthread mutex.

## Bibliography

<table> <tbody> <tr> <td> \[ <a> ISO/IEC 9899:2011 </a> \] </td> <td> 7.26.4.1, "The <code>mtx_destroy</code> Function" </td> </tr> </tbody> </table>


## Implementation notes

None

## References

* CERT-C: [CON31-C: Do not destroy a mutex while it is locked](https://wiki.sei.cmu.edu/confluence/display/c)
22 changes: 22 additions & 0 deletions c/cert/src/rules/CON31-C/DoNotDestroyAMutexWhileItIsLocked.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/**
* @id c/cert/do-not-destroy-a-mutex-while-it-is-locked
* @name CON31-C: Do not destroy a mutex while it is locked
* @description Calling delete on a locked mutex removes protections around shared resources.
* @kind problem
* @precision high
* @problem.severity error
* @tags external/cert/id/con31-c
* correctness
* concurrency
* external/cert/obligation/rule
*/

import cpp
import codingstandards.c.cert
import codingstandards.cpp.rules.donotdestroyamutexwhileitislocked.DoNotDestroyAMutexWhileItIsLocked

class DoNotDestroyAMutexWhileItIsLockedQuery extends DoNotDestroyAMutexWhileItIsLockedSharedQuery {
DoNotDestroyAMutexWhileItIsLockedQuery() {
this = Concurrency3Package::doNotDestroyAMutexWhileItIsLockedQuery()
}
}
Loading