Skip to content

Commit 1161be0

Browse files
s-samadis-samadi
committed
fixed formatting issues
1 parent 1a1dbba commit 1161be0

File tree

13 files changed

+489
-108
lines changed

13 files changed

+489
-108
lines changed

‎c/cert/src/rules/CON34-C/AppropriateThreadObjectStorageDurations.md.tmp‎

Lines changed: 364 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,364 @@
1+
2+
## Description
3+
Accessing the automatic or thread-local variables of one thread from another thread is [implementation-defined behavior](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-implementation-definedbehavior) and can cause invalid memory accesses because the execution of threads can be interwoven within the constraints of the synchronization model. As a result, the referenced stack frame or thread-local variable may no longer be valid when another thread tries to access it. Shared static variables can be protected by thread synchronization mechanisms.
4+
5+
However, automatic (local) variables cannot be shared in the same manner because the referenced stack frame's thread would need to stop executing, or some other mechanism must be employed to ensure that the referenced stack frame is still valid. Do not access automatic or thread-local objects from a thread other than the one with which the object is associated. See [DCL30-C. Declare objects with appropriate storage durations](https://wiki.sei.cmu.edu/confluence/display/c/DCL30-C.+Declare+objects+with+appropriate+storage+durations) for information on how to declare objects with appropriate storage durations when data is not being shared between threads.
6+
7+
Noncompliant Code Example (Automatic Storage Duration)
8+
9+
This noncompliant code example passes the address of a variable to a child thread, which prints it out. The variable has automatic storage duration. Depending on the execution order, the child thread might reference the variable after the variable's lifetime in the parent thread. This would cause the child thread to access an invalid memory location.
10+
11+
```cpp
12+
#include <threads.h>
13+
#include <stdio.h>
14+
15+
int child_thread(void *val) {
16+
int *res = (int *)val;
17+
printf("Result: %d\n", *res);
18+
return 0;
19+
}
20+
21+
void create_thread(thrd_t *tid) {
22+
int val = 1;
23+
if (thrd_success != thrd_create(tid, child_thread, &val)) {
24+
/* Handle error */
25+
}
26+
}
27+
28+
int main(void) {
29+
thrd_t tid;
30+
create_thread(&tid);
31+
32+
if (thrd_success != thrd_join(tid, NULL)) {
33+
/* Handle error */
34+
}
35+
return 0;
36+
}
37+
38+
```
39+
40+
## Noncompliant Code Example (Automatic Storage Duration)
41+
One practice is to ensure that all objects with automatic storage duration shared between threads are declared such that their lifetime extends past the lifetime of the threads. This can be accomplished using a thread synchronization mechanism, such as `thrd_join()`. In this code example, `val` is declared in `main()`, where `thrd_join()` is called. Because the parent thread waits until the child thread completes before continuing its execution, the shared objects have a lifetime at least as great as the thread.
42+
43+
```cpp
44+
#include <threads.h>
45+
#include <stdio.h>
46+
47+
int child_thread(void *val) {
48+
int *result = (int *)val;
49+
printf("Result: %d\n", *result); /* Correctly prints 1 */
50+
return 0;
51+
}
52+
53+
void create_thread(thrd_t *tid, int *val) {
54+
if (thrd_success != thrd_create(tid, child_thread, val)) {
55+
/* Handle error */
56+
}
57+
}
58+
59+
int main(void) {
60+
int val = 1;
61+
thrd_t tid;
62+
create_thread(&tid, &val);
63+
if (thrd_success != thrd_join(tid, NULL)) {
64+
/* Handle error */
65+
}
66+
return 0;
67+
}
68+
```
69+
70+
##
71+
However, the C Standard, 6.2.4 paragraphs 4 and 5 \[[ISO/IEC 9899:2011](https://wiki.sei.cmu.edu/confluence/display/c/AA.+Bibliography)\], states:
72+
73+
> The result of attempting to indirectly access an object with thread storage duration from a thread other than the one with which the object is associated is implementation-defined. . . .
74+
75+
76+
The result of attempting to indirectly access an object with automatic storage duration from a thread other than the one with which the object is associated is implementation-defined.
77+
78+
Therefore this example relies on [implementation-defined behavior](https://wiki.sei.cmu.edu/confluence/display/c/BB.+Definitions#BB.Definitions-implementation-definedbehavior) and is nonportable.
79+
80+
## Compliant Solution (Static Storage Duration)
81+
This compliant solution stores the value in an object having static storage duration. The lifetime of this object is the entire execution of the program; consequently, it can be safely accessed by any thread.
82+
83+
```cpp
84+
#include <threads.h>
85+
#include <stdio.h>
86+
87+
int child_thread(void *v) {
88+
int *result = (int *)v;
89+
printf("Result: %d\n", *result); /* Correctly prints 1 */
90+
return 0;
91+
}
92+
93+
void create_thread(thrd_t *tid) {
94+
static int val = 1;
95+
if (thrd_success != thrd_create(tid, child_thread, &val)) {
96+
/* Handle error */
97+
}
98+
}
99+
100+
int main(void) {
101+
thrd_t tid;
102+
create_thread(&tid);
103+
if (thrd_success != thrd_join(tid, NULL)) {
104+
/* Handle error */
105+
}
106+
return 0;
107+
}
108+
109+
```
110+
111+
## Compliant Solution (Allocated Storage Duration)
112+
This compliant solution stores the value passed to the child thread in a dynamically allocated object. Because this object will persist until explicitly freed, the child thread can safely access its value.
113+
114+
```cpp
115+
#include <threads.h>
116+
#include <stdio.h>
117+
#include <stdlib.h>
118+
119+
int child_thread(void *val) {
120+
int *result = (int *)val;
121+
printf("Result: %d\n", *result); /* Correctly prints 1 */
122+
return 0;
123+
}
124+
125+
void create_thread(thrd_t *tid, int *value) {
126+
*value = 1;
127+
if (thrd_success != thrd_create(tid, child_thread,
128+
value)) {
129+
/* Handle error */
130+
}
131+
}
132+
133+
int main(void) {
134+
thrd_t tid;
135+
int *value = (int *)malloc(sizeof(int));
136+
if (!value) {
137+
/* Handle error */
138+
}
139+
create_thread(&tid, value);
140+
if (thrd_success != thrd_join(tid, NULL)) {
141+
/* Handle error */
142+
}
143+
free(value);
144+
return 0;
145+
}
146+
147+
```
148+
149+
## Noncompliant Code Example (Thread-Specific Storage)
150+
In this noncompliant code example, the value is stored in thread-specific storage of the parent thread. However, because thread-specific data is available only to the thread that stores it, the `child_thread()` function will set `result` to a null value.
151+
152+
```cpp
153+
#include <threads.h>
154+
#include <stdio.h>
155+
#include <stdlib.h>
156+
157+
static tss_t key;
158+
159+
int child_thread(void *v) {
160+
void *result = tss_get(*(tss_t *)v);
161+
printf("Result: %d\n", *(int *)result);
162+
return 0;
163+
}
164+
165+
int create_thread(void *thrd) {
166+
int *val = (int *)malloc(sizeof(int));
167+
if (val == NULL) {
168+
/* Handle error */
169+
}
170+
*val = 1;
171+
if (thrd_success != tss_set(key, val)) {
172+
/* Handle error */
173+
}
174+
if (thrd_success != thrd_create((thrd_t *)thrd,
175+
child_thread, &key)) {
176+
/* Handle error */
177+
}
178+
return 0;
179+
}
180+
181+
int main(void) {
182+
thrd_t parent_tid, child_tid;
183+
184+
if (thrd_success != tss_create(&key, free)) {
185+
/* Handle error */
186+
}
187+
if (thrd_success != thrd_create(&parent_tid, create_thread,
188+
&child_tid)) {
189+
/* Handle error */
190+
}
191+
if (thrd_success != thrd_join(parent_tid, NULL)) {
192+
/* Handle error */
193+
}
194+
if (thrd_success != thrd_join(child_tid, NULL)) {
195+
/* Handle error */
196+
}
197+
tss_delete(key);
198+
return 0;
199+
}
200+
```
201+
202+
## Compliant Solution (Thread-Specific Storage)
203+
This compliant solution illustrates how thread-specific storage can be combined with a call to a thread synchronization mechanism, such as `thrd_join()`. Because the parent thread waits until the child thread completes before continuing its execution, the child thread is guaranteed to access a valid live object.
204+
205+
```cpp
206+
#include <threads.h>
207+
#include <stdio.h>
208+
#include <stdlib.h>
209+
210+
static tss_t key;
211+
212+
int child_thread(void *v) {
213+
int *result = v;
214+
printf("Result: %d\n", *result); /* Correctly prints 1 */
215+
return 0;
216+
}
217+
218+
int create_thread(void *thrd) {
219+
int *val = (int *)malloc(sizeof(int));
220+
if (val == NULL) {
221+
/* Handle error */
222+
}
223+
*val = 1;
224+
if (thrd_success != tss_set(key, val)) {
225+
/* Handle error */
226+
}
227+
/* ... */
228+
void *v = tss_get(key);
229+
if (thrd_success != thrd_create((thrd_t *)thrd,
230+
child_thread, v)) {
231+
/* Handle error */
232+
}
233+
return 0;
234+
}
235+
236+
int main(void) {
237+
thrd_t parent_tid, child_tid;
238+
239+
if (thrd_success != tss_create(&key, free)) {
240+
/* Handle error */
241+
}
242+
if (thrd_success != thrd_create(&parent_tid, create_thread,
243+
&child_tid)) {
244+
/* Handle error */
245+
}
246+
if (thrd_success != thrd_join(parent_tid, NULL)) {
247+
/* Handle error */
248+
}
249+
if (thrd_success != thrd_join(child_tid, NULL)) {
250+
/* Handle error */
251+
}
252+
tss_delete(key);
253+
return 0;
254+
}
255+
```
256+
This compliant solution uses pointer-to-integer and integer-to-pointer conversions, which have implementation-defined behavior. (See [INT36-C. Converting a pointer to integer or integer to pointer](https://wiki.sei.cmu.edu/confluence/display/c/INT36-C.+Converting+a+pointer+to+integer+or+integer+to+pointer).)
257+
258+
## Compliant Solution (Thread-Local Storage, Windows, Visual Studio)
259+
Similar to the preceding compliant solution, this compliant solution uses thread-local storage combined with thread synchronization to ensure the child thread is accessing a valid live object. It uses the Visual Studio–specific [__declspec(thread)](http://msdn.microsoft.com/en-us/library/9w1sdazb.aspx) language extension to provide the thread-local storage and the `[WaitForSingleObject()](http://msdn.microsoft.com/en-us/library/windows/desktop/ms687032(v=vs.85).aspx)` API to provide the synchronization.
260+
261+
```cpp
262+
#include <Windows.h>
263+
#include <stdio.h>
264+
265+
DWORD WINAPI child_thread(LPVOID v) {
266+
int *result = (int *)v;
267+
printf("Result: %d\n", *result); /* Correctly prints 1 */
268+
return NULL;
269+
}
270+
271+
int create_thread(HANDLE *tid) {
272+
/* Declare val as a thread-local value */
273+
__declspec(thread) int val = 1;
274+
*tid = create_thread(NULL, 0, child_thread, &val, 0, NULL);
275+
return *tid == NULL;
276+
}
277+
278+
int main(void) {
279+
HANDLE tid;
280+
281+
if (create_thread(&tid)) {
282+
/* Handle error */
283+
}
284+
285+
if (WAIT_OBJECT_0 != WaitForSingleObject(tid, INFINITE)) {
286+
/* Handle error */
287+
}
288+
CloseHandle(tid);
289+
290+
return 0;
291+
}
292+
293+
```
294+
295+
## Noncompliant Code Example (OpenMP, parallel)
296+
It is important to note that local data can be used securely with threads when using other thread interfaces, so the programmer need not always copy data into nonlocal memory when sharing data with threads. For example, the `shared` keyword in *<sup>®<a>The OpenMP API Specification for Parallel Programming</a></sup>* \[[OpenMP](http://openmp.org/wp/)\] can be used in combination with OpenMP's threading interface to share local memory without having to worry about whether local automatic variables remain valid.
297+
298+
In this noncompliant code example, a variable `j` is declared outside a `parallel` `#pragma` and not listed as a private variable. In OpenMP, variables outside a `parallel #pragma` are shared unless designated as `private`.
299+
300+
```cpp
301+
#include <omp.h>
302+
#include <stdio.h>
303+
304+
int main(void) {
305+
int j = 0;
306+
#pragma omp parallel
307+
{
308+
int t = omp_get_thread_num();
309+
printf("Running thread - %d\n", t);
310+
for (int i = 0; i < 5050; i++) {
311+
j++; /* j not private; could be a race condition */
312+
}
313+
printf("Just ran thread - %d\n", t);
314+
printf("loop count %d\n", j);
315+
}
316+
return 0;
317+
}
318+
```
319+
320+
## Compliant Solution (OpenMP, parallel, private)
321+
In this compliant solution, the variable `j` is declared outside of the `parallel` `#pragma` but is explicitly labeled as `private`:
322+
323+
```cpp
324+
#include <omp.h>
325+
#include <stdio.h>
326+
327+
int main(void) {
328+
int j = 0;
329+
#pragma omp parallel private(j)
330+
{
331+
int t = omp_get_thread_num();
332+
printf("Running thread - %d\n", t);
333+
for (int i = 0; i < 5050; i++) {
334+
j++;
335+
}
336+
printf("Just ran thread - %d\n", t);
337+
printf("loop count %d\n", j);
338+
}
339+
return 0;
340+
}
341+
```
342+
343+
## Risk Assessment
344+
Threads that reference the stack of other threads can potentially overwrite important information on the stack, such as function pointers and return addresses. The compiler may not generate warnings if the programmer allows one thread to access another thread's local variables, so a programmer may not catch a potential error at compile time. The remediation cost for this error is high because analysis tools have difficulty diagnosing problems with concurrency and race conditions.
345+
346+
<table> <tbody> <tr> <th> Recommendation </th> <th> Severity </th> <th> Likelihood </th> <th> Remediation Cost </th> <th> Priority </th> <th> Level </th> </tr> <tr> <td> CON34-C </td> <td> Medium </td> <td> Probable </td> <td> High </td> <td> <strong>P4</strong> </td> <td> <strong>L3</strong> </td> </tr> </tbody> </table>
347+
348+
349+
## Automated Detection
350+
<table> <tbody> <tr> <th> Tool </th> <th> Version </th> <th> Checker </th> <th> Description </th> </tr> <tr> <td> <a> CodeSonar </a> </td> <td> 7.2p0 </td> <td> <strong>CONCURRENCY.LOCALARG</strong> </td> <td> Local Variable Passed to Thread </td> </tr> <tr> <td> <a> Helix QAC </a> </td> <td> 2022.4 </td> <td> <strong>DF4926, DF4927, DF4928</strong> </td> <td> </td> </tr> <tr> <td> <a> Parasoft C/C++test </a> </td> <td> 2022.2 </td> <td> <strong>CERT_C-CON34-a</strong> </td> <td> Declare objects shared between POSIX threads with appropriate storage durations </td> </tr> <tr> <td> <a> Polyspace Bug Finder </a> </td> <td> R2022b </td> <td> <a> CERT C: Rule CON34-C </a> </td> <td> Checks for automatic or thread local variable escaping from a C11 thread (rule fully covered) </td> </tr> <tr> <td> <a> PRQA QA-C </a> </td> <td> 9.7 </td> <td> <strong>4926, 4927, 4928</strong> </td> <td> Enforced by QAC </td> </tr> </tbody> </table>
351+
352+
353+
## Related Vulnerabilities
354+
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+CON34-C).
355+
356+
## Related Guidelines
357+
[Key here](https://wiki.sei.cmu.edu/confluence/display/c/How+this+Coding+Standard+is+Organized#HowthisCodingStandardisOrganized-RelatedGuidelines) (explains table format and definitions)
358+
359+
<table> <tbody> <tr> <th> Taxonomy </th> <th> Taxonomy item </th> <th> Relationship </th> </tr> <tr> <td> <a> CERT C Secure Coding Standard </a> </td> <td> <a> DCL30-C. Declare objects with appropriate storage durations </a> </td> <td> Prior to 2018-01-12: CERT: Unspecified Relationship </td> </tr> </tbody> </table>
360+
361+
362+
## Bibliography
363+
<table> <tbody> <tr> <td> \[ <a> ISO/IEC 9899:2011 </a> \] </td> <td> 6.2.4, "Storage Durations of Objects" </td> </tr> <tr> <td> \[ <a> OpenMP </a> \] </td> <td> <em> <sup> ® <a> The OpenMP API Specification for Parallel Programming </a> </sup> </em> </td> </tr> </tbody> </table>
364+

‎c/misra/src/rules/RULE-15-3/GotoLabelBlockCondition.ql‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,6 @@ Stmt getNextStmt(ControlFlowNode node) {
2727
)
2828
}
2929

30-
Stmt getPreviousStmt(Stmt s) { s = getNextStmt(result) }
31-
3230
SwitchCase getSwitchCase(Stmt stmt) {
3331
exists(int index, SwitchStmt switch |
3432
getStmtInSwitch(switch, stmt, index) and getStmtInSwitch(switch, result, index - 1)
@@ -52,15 +50,6 @@ int statementDepth(Stmt statement) {
5250
statementDepth(statement.getParent()) + 1 = result
5351
}
5452

55-
predicate test(GotoStmt goto, Stmt target, int m, int n) {
56-
statementDepth(goto) = m and
57-
target = goto.getTarget() and
58-
statementDepth(target) = n and
59-
isPartOfSwitch(goto) and
60-
getSwitchCase(goto) = getSwitchCase(target) and
61-
m = n
62-
}
63-
6453
from GotoStmt goto, Stmt target, int gotoDepth, int targetDepth
6554
where
6655
not isExcluded(goto, Statements2Package::gotoLabelBlockConditionQuery()) and

0 commit comments

Comments
 (0)