Skip to content

Commit 05f017c

Browse files
authored
Merge branch 'main' into mbaluda/IO4
2 parents 4ca36af + d2e0cbc commit 05f017c

File tree

151 files changed

+747
-226
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

151 files changed

+747
-226
lines changed

.vscode/tasks.json

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,28 @@
9696
},
9797
"problemMatcher": []
9898
},
99+
{
100+
"label": "📖 Standards Automation: Generate Help Files for CERT rules",
101+
"type": "shell",
102+
"windows": {
103+
"command": ".${pathSeparator}scripts${pathSeparator}.venv${pathSeparator}Scripts${pathSeparator}python.exe scripts${pathSeparator}help${pathSeparator}cert-help-extraction.py ${input:rule}"
104+
},
105+
"linux": {
106+
"command": ".${pathSeparator}scripts${pathSeparator}.venv${pathSeparator}bin${pathSeparator}python3 scripts${pathSeparator}help${pathSeparator}cert-help-extraction.py ${input:rule}"
107+
},
108+
"osx": {
109+
"command": ".${pathSeparator}scripts${pathSeparator}.venv${pathSeparator}bin${pathSeparator}python3 scripts${pathSeparator}help${pathSeparator}cert-help-extraction.py ${input:rule}"
110+
},
111+
"presentation": {
112+
"reveal": "always",
113+
"panel": "new",
114+
"focus": true
115+
},
116+
"runOptions": {
117+
"reevaluateOnRerun": false
118+
},
119+
"problemMatcher": []
120+
},
99121
{
100122
"label": "🧪 Standards Automation: Build Case Test DB",
101123
"type": "shell",

c/cert/src/rules/CON33-C/RaceConditionsWhenUsingLibraryFunctions.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ void f(FILE *fp) {
3636
```
3737
This code first sets `errno` to 0 to comply with [ERR30-C. Take care when reading errno](https://wiki.sei.cmu.edu/confluence/display/c/ERR30-C.+Take+care+when+reading+errno).
3838
39-
## Compliant Solution (Annex K, strerror_s())
39+
## Compliant Solution (Annex K, strerror_s())
4040
4141
This compliant solution uses the `strerror_s()` function from Annex K of the C Standard, which has the same functionality as `strerror()` but guarantees thread-safety:
4242
@@ -45,7 +45,7 @@ This compliant solution uses the `strerror_s()` function from Annex K of the C S
4545
#include <errno.h>
4646
#include <stdio.h>
4747
#include <string.h>
48-
 
48+
4949
enum { BUFFERSIZE = 64 };
5050
void f(FILE *fp) {
5151
fpos_t pos;
@@ -60,9 +60,9 @@ void f(FILE *fp) {
6060
}
6161
}
6262
```
63-
Because Annex K is optional, `strerror_s()` may not be available in all implementations. 
63+
Because Annex K is optional, `strerror_s()` may not be available in all implementations.
6464

65-
## Compliant Solution (POSIX, strerror_r())
65+
## Compliant Solution (POSIX, strerror_r())
6666

6767
This compliant solution uses the POSIX `strerror_r()` function, which has the same functionality as `strerror()` but guarantees thread safety:
6868

@@ -72,7 +72,7 @@ This compliant solution uses the POSIX `strerror_r()` function, which has the sa
7272
#include <string.h>
7373

7474
enum { BUFFERSIZE = 64 };
75-
 
75+
7676
void f(FILE *fp) {
7777
fpos_t pos;
7878
errno = 0;
Lines changed: 245 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,245 @@
1+
# CON35-C: Avoid deadlock by locking in a predefined order
2+
3+
This query implements the CERT-C rule CON35-C:
4+
5+
> Avoid deadlock by locking in a predefined order
6+
7+
8+
## Description
9+
10+
Mutexes are used to prevent multiple threads from causing a data race by accessing shared resources at the same time. Sometimes, when locking mutexes, multiple threads hold each other's lock, and the program consequently deadlocks. Four conditions are required for deadlock to occur:
11+
12+
* Mutual exclusion
13+
* Hold and wait
14+
* No preemption
15+
* Circular wait
16+
Deadlock needs all four conditions, so preventing deadlock requires preventing any one of the four conditions. One simple solution is to lock the mutexes in a predefined order, which prevents circular wait.
17+
18+
## Noncompliant Code Example
19+
20+
The behavior of this noncompliant code example depends on the runtime environment and the platform's scheduler. The program is susceptible to deadlock if thread `thr1 `attempts to lock `ba2`'s mutex at the same time thread `thr2` attempts to lock `ba1`'s mutex in the `deposit()` function.
21+
22+
```cpp
23+
#include <stdlib.h>
24+
#include <threads.h>
25+
26+
typedef struct {
27+
int balance;
28+
mtx_t balance_mutex;
29+
} bank_account;
30+
31+
typedef struct {
32+
bank_account *from;
33+
bank_account *to;
34+
int amount;
35+
} transaction;
36+
37+
void create_bank_account(bank_account **ba,
38+
int initial_amount) {
39+
bank_account *nba = (bank_account *)malloc(
40+
sizeof(bank_account)
41+
);
42+
if (nba == NULL) {
43+
/* Handle error */
44+
}
45+
46+
nba->balance = initial_amount;
47+
if (thrd_success
48+
!= mtx_init(&nba->balance_mutex, mtx_plain)) {
49+
/* Handle error */
50+
}
51+
52+
*ba = nba;
53+
}
54+
55+
int deposit(void *ptr) {
56+
transaction *args = (transaction *)ptr;
57+
58+
if (thrd_success != mtx_lock(&args->from->balance_mutex)) {
59+
/* Handle error */
60+
}
61+
62+
/* Not enough balance to transfer */
63+
if (args->from->balance < args->amount) {
64+
if (thrd_success
65+
!= mtx_unlock(&args->from->balance_mutex)) {
66+
/* Handle error */
67+
}
68+
return -1; /* Indicate error */
69+
}
70+
if (thrd_success != mtx_lock(&args->to->balance_mutex)) {
71+
/* Handle error */
72+
}
73+
74+
args->from->balance -= args->amount;
75+
args->to->balance += args->amount;
76+
77+
if (thrd_success
78+
!= mtx_unlock(&args->from->balance_mutex)) {
79+
/* Handle error */
80+
}
81+
82+
if (thrd_success
83+
!= mtx_unlock(&args->to->balance_mutex)) {
84+
/* Handle error */
85+
}
86+
87+
free(ptr);
88+
return 0;
89+
}
90+
91+
int main(void) {
92+
thrd_t thr1, thr2;
93+
transaction *arg1;
94+
transaction *arg2;
95+
bank_account *ba1;
96+
bank_account *ba2;
97+
98+
create_bank_account(&ba1, 1000);
99+
create_bank_account(&ba2, 1000);
100+
101+
arg1 = (transaction *)malloc(sizeof(transaction));
102+
if (arg1 == NULL) {
103+
/* Handle error */
104+
}
105+
arg2 = (transaction *)malloc(sizeof(transaction));
106+
if (arg2 == NULL) {
107+
/* Handle error */
108+
}
109+
arg1->from = ba1;
110+
arg1->to = ba2;
111+
arg1->amount = 100;
112+
113+
arg2->from = ba2;
114+
arg2->to = ba1;
115+
arg2->amount = 100;
116+
117+
/* Perform the deposits */
118+
if (thrd_success
119+
!= thrd_create(&thr1, deposit, (void *)arg1)) {
120+
/* Handle error */
121+
}
122+
if (thrd_success
123+
!= thrd_create(&thr2, deposit, (void *)arg2)) {
124+
/* Handle error */
125+
}
126+
return 0;
127+
}
128+
```
129+
130+
## Compliant Solution
131+
132+
This compliant solution eliminates the circular wait condition by establishing a predefined order for locking in the `deposit()` function. Each thread will lock on the basis of the `bank_account` ID, which is set when the `bank_account struct` is initialized.
133+
134+
```cpp
135+
#include <stdlib.h>
136+
#include <threads.h>
137+
138+
typedef struct {
139+
int balance;
140+
mtx_t balance_mutex;
141+
142+
/* Should not change after initialization */
143+
unsigned int id;
144+
} bank_account;
145+
146+
typedef struct {
147+
bank_account *from;
148+
bank_account *to;
149+
int amount;
150+
} transaction;
151+
152+
unsigned int global_id = 1;
153+
154+
void create_bank_account(bank_account **ba,
155+
int initial_amount) {
156+
bank_account *nba = (bank_account *)malloc(
157+
sizeof(bank_account)
158+
);
159+
if (nba == NULL) {
160+
/* Handle error */
161+
}
162+
163+
nba->balance = initial_amount;
164+
if (thrd_success
165+
!= mtx_init(&nba->balance_mutex, mtx_plain)) {
166+
/* Handle error */
167+
}
168+
169+
nba->id = global_id++;
170+
*ba = nba;
171+
}
172+
173+
int deposit(void *ptr) {
174+
transaction *args = (transaction *)ptr;
175+
int result = -1;
176+
mtx_t *first;
177+
mtx_t *second;
178+
179+
if (args->from->id == args->to->id) {
180+
return -1; /* Indicate error */
181+
}
182+
183+
/* Ensure proper ordering for locking */
184+
if (args->from->id < args->to->id) {
185+
first = &args->from->balance_mutex;
186+
second = &args->to->balance_mutex;
187+
} else {
188+
first = &args->to->balance_mutex;
189+
second = &args->from->balance_mutex;
190+
}
191+
if (thrd_success != mtx_lock(first)) {
192+
/* Handle error */
193+
}
194+
if (thrd_success != mtx_lock(second)) {
195+
/* Handle error */
196+
}
197+
198+
/* Not enough balance to transfer */
199+
if (args->from->balance >= args->amount) {
200+
args->from->balance -= args->amount;
201+
args->to->balance += args->amount;
202+
result = 0;
203+
}
204+
205+
if (thrd_success != mtx_unlock(second)) {
206+
/* Handle error */
207+
}
208+
if (thrd_success != mtx_unlock(first)) {
209+
/* Handle error */
210+
}
211+
free(ptr);
212+
return result;
213+
}
214+
```
215+
216+
## Risk Assessment
217+
218+
Deadlock prevents multiple threads from progressing, halting program execution. A [denial-of-service attack](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-denial-of-service) is possible if the attacker can create the conditions for deadlock.
219+
220+
<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> CON35-C </td> <td> Low </td> <td> Probable </td> <td> Medium </td> <td> <strong>P4</strong> </td> <td> <strong>L3</strong> </td> </tr> </tbody> </table>
221+
222+
223+
## Related Vulnerabilities
224+
225+
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+CON35-C).
226+
227+
## Automated Detection
228+
229+
<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> <strong>deadlock</strong> </td> <td> Supported by sound analysis (deadlock alarm) </td> </tr> <tr> <td> <a> CodeSonar </a> </td> <td> 7.0p0 </td> <td> <strong>CONCURRENCY.LOCK.ORDER</strong> </td> <td> Conflicting lock order </td> </tr> <tr> <td> <a> Coverity </a> </td> <td> 2017.07 </td> <td> <strong>ORDER_REVERSAL</strong> </td> <td> Fully implemented </td> </tr> <tr> <td> <a> Helix QAC </a> </td> <td> 2022.2 </td> <td> <strong>C1772, C1773</strong> </td> <td> </td> </tr> <tr> <td> <a> Klocwork </a> </td> <td> 2022.2 </td> <td> <strong>CONC.DL</strong> <strong>CONC.NO_UNLOCK</strong> </td> <td> </td> </tr> <tr> <td> <a> Parasoft C/C++test </a> </td> <td> 2022.1 </td> <td> <strong>CERT_C-CON35-a</strong> </td> <td> Avoid double locking </td> </tr> <tr> <td> <a> PC-lint Plus </a> </td> <td> 1.4 </td> <td> <strong>2462</strong> </td> <td> Fully supported </td> </tr> <tr> <td> <a> Polyspace Bug Finder </a> </td> <td> R2022a </td> <td> <a> CERT C: Rule CON35-C </a> </td> <td> Checks for deadlock (rule fully covered) </td> </tr> <tr> <td> <a> PRQA QA-C </a> </td> <td> 9.7 </td> <td> <strong>1772,1773</strong> </td> <td> Enforced by MTA </td> </tr> </tbody> </table>
230+
231+
232+
## Related Guidelines
233+
234+
[Key here](https://wiki.sei.cmu.edu/confluence/display/c/How+this+Coding+Standard+is+Organized#HowthisCodingStandardisOrganized-RelatedGuidelines) (explains table format and definitions)
235+
236+
<table> <tbody> <tr> <th> Taxonomy </th> <th> Taxonomy item </th> <th> Relationship </th> </tr> <tr> <td> <a> CERT Oracle Secure Coding Standard for Java </a> </td> <td> <a> LCK07-J. Avoid deadlock by requesting and releasing locks in the same order </a> </td> <td> Prior to 2018-01-12: CERT: Unspecified Relationship </td> </tr> </tbody> </table>
237+
238+
239+
## Implementation notes
240+
241+
None
242+
243+
## References
244+
245+
* CERT-C: [CON35-C: Avoid deadlock by locking in a predefined order](https://wiki.sei.cmu.edu/confluence/display/c)

0 commit comments

Comments
 (0)