Skip to content

Commit e31a1e8

Browse files
committed
Add CodeQL check to detect cause of issue openzfs#12014
Signed-off-by: Richard Yao <[email protected]>
1 parent d5616ad commit e31a1e8

File tree

2 files changed

+82
-0
lines changed

2 files changed

+82
-0
lines changed

.github/codeql-cpp.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,4 @@ name: "Custom CodeQL Analysis"
22

33
queries:
44
- uses: ./.github/codeql/custom-queries/cpp/deprecatedFunctionUsage.ql
5+
- uses: ./.github/codeql/custom-queries/cpp/dsl_dataset_hold_rele_mismatch.ql
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
/**
2+
* @name Detect mismatched dsl_dataset_hold/_rele pairs
3+
* @description Flags instances of issue #12014 where
4+
* - a dataset held with dsl_dataset_hold_obj() ends up in dsl_dataset_rele_flags(), or
5+
* - a dataset held with dsl_dataset_hold_obj_flags() ends up in dsl_dataset_rele().
6+
* @kind path-problem
7+
* @severity error
8+
* @tags correctness
9+
* @id cpp/dsl_dataset_hold_rele_mismatch
10+
*/
11+
12+
import cpp
13+
import dataflow
14+
15+
/**
16+
* Taint any `&X` passed to dsl_dataset_hold_obj(..., &X)
17+
* and look for it in dsl_dataset_rele_flags(X, ...).
18+
*/
19+
class HoldObjToReleFlagsConfig extends DataFlow::Configuration {
20+
HoldObjToReleFlagsConfig() { this = "HoldObjToReleFlags" }
21+
22+
override predicate isSource(Node n) {
23+
exists(FunctionCall hold, AddressOfExpr ao, DeclRefExpr dre |
24+
hold.getTarget().getName() = "dsl_dataset_hold_obj" and
25+
// the 4th argument is “&X”
26+
ao = hold.getArgument(3).(AddressOfExpr) and
27+
// its operand is the decl-ref of X
28+
dre = ao.getOperand().(DeclRefExpr) and
29+
n.getNode() = dre
30+
)
31+
}
32+
33+
override predicate isSink(Node n) {
34+
exists(FunctionCall rele, DeclRefExpr dre |
35+
rele.getTarget().getName() = "dsl_dataset_rele_flags" and
36+
// the 1st argument is X
37+
dre = rele.getArgument(0).(DeclRefExpr) and
38+
n.getNode() = dre
39+
)
40+
}
41+
}
42+
43+
/**
44+
* Taint any `&X` passed to dsl_dataset_hold_obj_flags(..., &X)
45+
* and look for it in dsl_dataset_rele(X, ...).
46+
*/
47+
class HoldObjFlagsToReleConfig extends DataFlow::Configuration {
48+
HoldObjFlagsToReleConfig() { this = "HoldObjFlagsToRele" }
49+
50+
override predicate isSource(Node n) {
51+
exists(FunctionCall hold, AddressOfExpr ao, DeclRefExpr dre |
52+
hold.getTarget().getName() = "dsl_dataset_hold_obj_flags" and
53+
// the 5th argument is “&X”
54+
ao = hold.getArgument(4).(AddressOfExpr) and
55+
dre = ao.getOperand().(DeclRefExpr) and
56+
n.getNode() = dre
57+
)
58+
}
59+
60+
override predicate isSink(Node n) {
61+
exists(FunctionCall rele, DeclRefExpr dre |
62+
rele.getTarget().getName() = "dsl_dataset_rele" and
63+
dre = rele.getArgument(0).(DeclRefExpr) and
64+
n.getNode() = dre
65+
)
66+
}
67+
}
68+
69+
// Single select covers both mismatch kinds, so no “multiple select” error.
70+
from DataFlow::PathNode src, DataFlow::PathNode snk, DataFlow::Configuration cfg
71+
where
72+
// either hold_obj → rele_flags or hold_obj_flags → rele
73+
(cfg instanceof HoldObjToReleFlagsConfig or cfg instanceof HoldObjFlagsToReleConfig) and
74+
cfg.hasPath(src, snk)
75+
select
76+
snk.getNode(),
77+
if(
78+
cfg instanceof HoldObjToReleFlagsConfig,
79+
"Pointer held by dsl_dataset_hold_obj() is flowing into dsl_dataset_rele_flags() — you should pair `hold_obj()` with `rele()`, not `rele_flags()`.",
80+
"Pointer held by dsl_dataset_hold_obj_flags() is flowing into dsl_dataset_rele() — you should pair `hold_obj_flags()` with `rele_flags()`, not `rele()`."
81+
)

0 commit comments

Comments
 (0)