From: Ondřej Surý <ondrej@sury.org>
Date: Mon, 9 Sep 2019 10:14:39 +0000 (+0200)
Subject: Add semantic patches to correctly check dns_name_copy(..., NULL) return code
X-Git-Tag: v9.15.5~11^2~8
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=406eba0c4125c99ec4c79839a8a890b812225ca4;p=thirdparty%2Fbind9.git

Add semantic patches to correctly check dns_name_copy(..., NULL) return code

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.
---

diff --git a/cocci/dns_name_copy-with-result.spatch b/cocci/dns_name_copy-with-result.spatch
new file mode 100644
index 00000000000..c4555c3a765
--- /dev/null
+++ b/cocci/dns_name_copy-with-result.spatch
@@ -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
index 00000000000..89e340cb4c4
--- /dev/null
+++ b/cocci/dns_name_copy.spatch
@@ -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);