]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
Add semantic patches to correctly check dns_name_copy(..., NULL) return code
authorOndřej Surý <ondrej@sury.org>
Mon, 9 Sep 2019 10:14:39 +0000 (12:14 +0200)
committerMark Andrews <marka@isc.org>
Tue, 1 Oct 2019 00:43:26 +0000 (10:43 +1000)
The dns_name_copy() function cannot fail gracefully when the last argument
(target) is NULL.  Add RUNTIME_CHECK()s around such calls.

The first semantic patch adds RUNTIME_CHECK() around any call that ignores the
return value and is very safe to apply.

The second semantic patch attempts to properly add RUNTIME_CHECK() to places
where the return value from `dns_name_copy()` is recorded into `result`
variable.  The result of this semantic patch needs to be reviewed by hand.

Both patches misses couple places where the code surrounding the
`dns_name_copy(..., NULL)` usage is more complicated and is better suited to be
fixed by a human being that understands the surrounding code.

cocci/dns_name_copy-with-result.spatch [new file with mode: 0644]
cocci/dns_name_copy.spatch [new file with mode: 0644]

diff --git a/cocci/dns_name_copy-with-result.spatch b/cocci/dns_name_copy-with-result.spatch
new file mode 100644 (file)
index 0000000..c4555c3
--- /dev/null
@@ -0,0 +1,30 @@
+@@
+expression V, E1, E2;
+statement S;
+@@
+
+- V = dns_name_copy(E1, E2, NULL);
+- if (V != ISC_R_SUCCESS) S
++ RUNTIME_CHECK(dns_name_copy(E1, E2, NULL) == ISC_R_SUCCESS);
+
+@@
+expression V, E1, E2;
+statement S1, S2;
+@@
+
+- V = dns_name_copy(E1, E2, NULL);
+- if (V == ISC_R_SUCCESS) S1 else S2;
++ RUNTIME_CHECK(dns_name_copy(E1, E2, NULL) == ISC_R_SUCCESS);
++ S2
+
+@@
+expression V, E1, E2;
+statement S1, S2;
+@@
+
+- V = dns_name_copy(E1, E2, NULL);
+- S1
+- if (V == ISC_R_SUCCESS) S2
++ RUNTIME_CHECK(dns_name_copy(E1, E2, NULL) == ISC_R_SUCCESS);
++ S1
++ S2
diff --git a/cocci/dns_name_copy.spatch b/cocci/dns_name_copy.spatch
new file mode 100644 (file)
index 0000000..89e340c
--- /dev/null
@@ -0,0 +1,30 @@
+@@
+expression E1, E2;
+@@
+
+- dns_name_copy(E1, E2, NULL);
++ RUNTIME_CHECK(dns_name_copy(E1, E2, NULL) == ISC_R_SUCCESS);
+
+@@
+expression E1, E2;
+@@
+
+- (void)dns_name_copy(E1, E2, NULL);
++ RUNTIME_CHECK(dns_name_copy(E1, E2, NULL) == ISC_R_SUCCESS);
+
+@@
+expression E1, E2;
+@@
+
+- return (dns_name_copy(E1, E2, NULL));
++ RUNTIME_CHECK(dns_name_copy(E1, E2, NULL) == ISC_R_SUCCESS);
++ return (ISC_R_SUCCESS);
+
+// ./bin/named/query.c processing broken with this rule, fix manually
+// @@
+// expression V, E1, E2;
+// @@
+//
+// - V = dns_name_copy(E1, E2, NULL);
+// - RUNTIME_CHECK(V == ISC_R_SUCCESS);
+// + RUNTIME_CHECK(dns_name_copy(E1, E2, NULL) == ISC_R_SUCCESS);