https://gcc.gnu.org/g:c6cf5789135236c5639075c8f235e7dd461b6ff6
commit r14-9625-gc6cf5789135236c5639075c8f235e7dd461b6ff6
Author: David Malcolm <dmalc...@redhat.com>
Date: Fri Mar 22 10:57:25 2024 -0400
analyzer: look through casts in taint sanitization [PR112974,PR112975]
PR analyzer/112974 and PR analyzer/112975 record false positives
from the analyzer's taint detection where sanitization of the form
if (VALUE CMP VALUE-OF-WIDER-TYPE)
happens, but wasn't being "noticed" by the taint checker, due to the
test being:
(WIDER_TYPE)VALUE CMP VALUE-OF-WIDER-TYPE
at the gimple level, and thus taint_state_machine recording
sanitization of (WIDER_TYPE)VALUE, but not of VALUE.
Fix by stripping casts in taint_state_machine::on_condition so that
the state machine records sanitization of the underlying value.
gcc/analyzer/ChangeLog:
PR analyzer/112974
PR analyzer/112975
* sm-taint.cc (taint_state_machine::on_condition): Strip away
casts before considering LHS and RHS, to increase the chance of
detecting places where sanitization of a value may have happened.
gcc/testsuite/ChangeLog:
PR analyzer/112974
PR analyzer/112975
* gcc.dg/plugin/plugin.exp (plugin_test_list): Add
taint-pr112974.c and taint-pr112975.c to analyzer_kernel_plugin.c.
* gcc.dg/plugin/taint-pr112974.c: New test.
* gcc.dg/plugin/taint-pr112975.c: New test.
Signed-off-by: David Malcolm <dmalc...@redhat.com>
Diff:
---
gcc/analyzer/sm-taint.cc | 8 ++++
gcc/testsuite/gcc.dg/plugin/plugin.exp | 2 +
gcc/testsuite/gcc.dg/plugin/taint-pr112974.c | 59 ++++++++++++++++++++++++++++
gcc/testsuite/gcc.dg/plugin/taint-pr112975.c | 53 +++++++++++++++++++++++++
4 files changed, 122 insertions(+)
diff --git a/gcc/analyzer/sm-taint.cc b/gcc/analyzer/sm-taint.cc
index c873c9ebd33..1d1e208fdf4 100644
--- a/gcc/analyzer/sm-taint.cc
+++ b/gcc/analyzer/sm-taint.cc
@@ -1109,6 +1109,14 @@ taint_state_machine::on_condition (sm_context *sm_ctxt,
return;
}
+ /* Strip away casts before considering LHS and RHS, to increase the
+ chance of detecting places where sanitization of a value may have
+ happened. */
+ if (const svalue *inner = lhs->maybe_undo_cast ())
+ lhs = inner;
+ if (const svalue *inner = rhs->maybe_undo_cast ())
+ rhs = inner;
+
// TODO
switch (op)
{
diff --git a/gcc/testsuite/gcc.dg/plugin/plugin.exp
b/gcc/testsuite/gcc.dg/plugin/plugin.exp
index c26dda1f324..933f9a5850b 100644
--- a/gcc/testsuite/gcc.dg/plugin/plugin.exp
+++ b/gcc/testsuite/gcc.dg/plugin/plugin.exp
@@ -172,6 +172,8 @@ set plugin_test_list [list \
taint-pr112850-too-complex.c \
taint-pr112850-unsanitized.c \
taint-pr112927.c \
+ taint-pr112974.c \
+ taint-pr112975.c \
taint-pr112977.c } \
{ analyzer_cpython_plugin.c \
cpython-plugin-test-no-Python-h.c \
diff --git a/gcc/testsuite/gcc.dg/plugin/taint-pr112974.c
b/gcc/testsuite/gcc.dg/plugin/taint-pr112974.c
new file mode 100644
index 00000000000..1af505326c7
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/plugin/taint-pr112974.c
@@ -0,0 +1,59 @@
+/* Reduced from false positive in Linux kernel in
+ drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c. */
+
+/* { dg-do compile } */
+/* { dg-options "-fanalyzer" } */
+/* { dg-require-effective-target analyzer } */
+
+typedef unsigned char __u8;
+typedef unsigned short __u16;
+extern unsigned int __max_logical_packages;
+extern unsigned long
+copy_from_user(void* to, const void* from, unsigned long n);
+extern unsigned long
+copy_to_user(void* to, const void* from, unsigned long n);
+struct isst_tpmi_instance_count
+{
+ __u8 socket_id;
+ __u8 count;
+ __u16 valid_mask;
+};
+struct tpmi_per_power_domain_info
+{
+ void* sst_base;
+};
+struct tpmi_sst_struct
+{
+ int number_of_power_domains;
+ struct tpmi_per_power_domain_info* power_domain_info;
+};
+struct tpmi_sst_common_struct
+{
+ int max_index;
+ struct tpmi_sst_struct** sst_inst;
+};
+static struct tpmi_sst_common_struct isst_common;
+int
+isst_if_get_tpmi_instance_count(void* argp)
+{
+ struct isst_tpmi_instance_count tpmi_inst;
+ struct tpmi_sst_struct* sst_inst;
+ int i;
+ if (copy_from_user(&tpmi_inst, argp, sizeof(tpmi_inst)))
+ return -14;
+ if (tpmi_inst.socket_id >= (__max_logical_packages))
+ return -22;
+ tpmi_inst.count =
+ isst_common.sst_inst[tpmi_inst.socket_id]->number_of_power_domains; /* {
dg-bogus "use of attacker-controlled value as offset without upper-bounds
checking" } */
+ sst_inst = isst_common.sst_inst[tpmi_inst.socket_id];
+ tpmi_inst.valid_mask = 0;
+ for (i = 0; i < sst_inst->number_of_power_domains; ++i) {
+ struct tpmi_per_power_domain_info* pd_info;
+ pd_info = &sst_inst->power_domain_info[i];
+ if (pd_info->sst_base)
+ tpmi_inst.valid_mask |= ((((1UL))) << (i));
+ }
+ if (copy_to_user(argp, &tpmi_inst, sizeof(tpmi_inst)))
+ return -14;
+ return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/plugin/taint-pr112975.c
b/gcc/testsuite/gcc.dg/plugin/taint-pr112975.c
new file mode 100644
index 00000000000..9f312cb3348
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/plugin/taint-pr112975.c
@@ -0,0 +1,53 @@
+/* Reduced from false positive in Linux kernel in
+ drivers/xen/privcmd.c. */
+
+/* { dg-do compile } */
+/* { dg-options "-fanalyzer" } */
+/* { dg-require-effective-target analyzer } */
+
+typedef __SIZE_TYPE__ size_t;
+typedef unsigned short __u16;
+typedef unsigned int gfp_t;
+void
+kfree(const void* objp);
+
+extern void *
+__attribute__((__alloc_size__(1, 2)))
+__attribute__((__malloc__))
+kcalloc(size_t n, size_t size, gfp_t flags);
+
+extern unsigned long
+copy_from_user(void*, const void*, unsigned long);
+
+typedef __u16 domid_t;
+struct privcmd_dm_op_buf
+{
+ void* uptr;
+ size_t size;
+};
+struct privcmd_dm_op
+{
+ domid_t dom;
+ __u16 num;
+};
+static unsigned int privcmd_dm_op_max_num = 16;
+long
+privcmd_ioctl_dm_op(void* udata)
+{
+ struct privcmd_dm_op kdata;
+ struct privcmd_dm_op_buf* kbufs;
+ if (copy_from_user(&kdata, udata, sizeof(kdata)))
+ return -14;
+ if (kdata.num == 0)
+ return 0;
+ if (kdata.num > privcmd_dm_op_max_num)
+ return -7;
+ kbufs =
+ kcalloc(kdata.num, /* { dg-bogus "attacker-controlled value" } */
+ sizeof(*kbufs),
+ (((gfp_t)(0x400u | 0x800u)) | ((gfp_t)0x40u) | ((gfp_t)0x80u)));
+ if (!kbufs)
+ return -12;
+ kfree(kbufs);
+ return 0;
+}
