[Bug analyzer/112975] New: -Wanalyzer-tainted-allocation-size false positive seen in Linux kernel's drivers/xen/privcmd.c

[Bug analyzer/112975] New: -Wanalyzer-tainted-allocation-size false positive seen in Linux kernel's drivers/xen/privcmd.c

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112975

            Bug ID: 112975
           Summary: -Wanalyzer-tainted-allocation-size false positive seen
                    in Linux kernel's drivers/xen/privcmd.c
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: analyzer
          Assignee: dmalcolm at gcc dot gnu.org
          Reporter: dmalcolm at gcc dot gnu.org
            Blocks: 106358
  Target Milestone: ---

In file included from drivers/xen/privcmd.c:15:
In function ‘kcalloc’,
    inlined from ‘privcmd_ioctl_dm_op’ at drivers/xen/privcmd.c:640:10:
./include/linux/slab.h:645:16: warning: use of attacker-controlled value as
allocation size without upper-bounds checking [CWE-789]
[-Wanalyzer-tainted-allocation-size]
  645 |         return kmalloc_array(n, size, flags | __GFP_ZERO);
      |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  ‘privcmd_ioctl’: events 1-4
    |
    |drivers/xen/privcmd.c:834:13:
    |  834 | static long privcmd_ioctl(struct file *file,
    |      |             ^~~~~~~~~~~~~
    |      |             |
    |      |             (1) entry to ‘privcmd_ioctl’
    |......
    |  840 |         switch (cmd) {
    |      |         ~~~~~~
    |      |         |
    |      |         (2) following ‘case 1069061:’ branch...
    |......
    |  857 |         case IOCTL_PRIVCMD_DM_OP:
    |      |         ~~~~ 
    |      |         |
    |      |         (3) ...to here
    |  858 |                 ret = privcmd_ioctl_dm_op(file, udata);
    |      |                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |                       |
    |      |                       (4) calling ‘privcmd_ioctl_dm_op’ from
‘privcmd_ioctl’
    |
    +--> ‘privcmd_ioctl_dm_op’: events 5-12
           |
           |  615 | static long privcmd_ioctl_dm_op(struct file *file, void
__user *udata)
           |      |             ^~~~~~~~~~~~~~~~~~~
           |      |             |
           |      |             (5) entry to ‘privcmd_ioctl_dm_op’
           |......
           |  627 |         if (copy_from_user(&kdata, udata, sizeof(kdata)))
           |      |            ~ 
           |      |            |
           |      |            (6) following ‘false’ branch (when ‘n == 0’)...
           |......
           |  631 |         if (data->domid != DOMID_INVALID && data->domid !=
kdata.dom)
           |      |         ~~   
           |      |         |
           |      |         (7) ...to here
           |......
           |  634 |         if (kdata.num == 0)
           |      |            ~ 
           |      |            |
           |      |            (8) following ‘false’ branch...
           |......
           |  637 |         if (kdata.num > privcmd_dm_op_max_num)
           |      |         ~~ ~ 
           |      |         |  |
           |      |         |  (10) following ‘false’ branch...
           |      |         (9) ...to here
           |......
           |  640 |         kbufs = kcalloc(kdata.num, sizeof(*kbufs),
GFP_KERNEL);
           |      |         ~~~~~   ~
           |      |         |       |
           |      |         |       (12) inlined call to ‘kcalloc’ from
‘privcmd_ioctl_dm_op’
           |      |         (11) ...to here
           |
           +--> ‘kcalloc’: event 13
                  |
                  |./include/linux/slab.h:645:16:
                  |  645 |         return kmalloc_array(n, size, flags |
__GFP_ZERO);
                  |      |               
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                  |      |                |
                  |      |                (13) use of attacker-controlled value
as allocation size without upper-bounds checking
                  |

...when the value is checked at (10).


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106358
[Bug 106358] [meta-bug] tracker bug for building the Linux kernel with
-fanalyzer

[Bug analyzer/112975] -Wanalyzer-tainted-allocation-size false positive seen in Linux kernel's drivers/xen/privcmd.c

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112975

--- Comment #1 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Created attachment 56857
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=56857&action=edit
Reduced reproducer (needs adding to plugin.exp)

[Bug analyzer/112975] [14 Regression] -Wanalyzer-tainted-allocation-size false positive seen in Linux kernel's drivers/xen/privcmd.c

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112975

David Malcolm <dmalcolm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2024-02-15

[Bug analyzer/112975] [14 Regression] -Wanalyzer-tainted-allocation-size false positive seen in Linux kernel's drivers/xen/privcmd.c

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112975

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|unknown                     |14.0
   Target Milestone|---                         |14.0

[Bug analyzer/112975] [14 Regression] -Wanalyzer-tainted-allocation-size false positive seen in Linux kernel's drivers/xen/privcmd.c

[law at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112975

Jeffrey A. Law <law at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |law at gcc dot gnu.org
           Priority|P3                          |P2

[Bug analyzer/112975] [14 Regression] -Wanalyzer-tainted-allocation-size false positive seen in Linux kernel's drivers/xen/privcmd.c

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112975

--- Comment #2 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by David Malcolm <dmalcolm@gcc.gnu.org>:

https://gcc.gnu.org/g:c6cf5789135236c5639075c8f235e7dd461b6ff6

commit r14-9625-gc6cf5789135236c5639075c8f235e7dd461b6ff6
Author: David Malcolm <dmalcolm@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 <dmalcolm@redhat.com>

[Bug analyzer/112975] [14 Regression] -Wanalyzer-tainted-allocation-size false positive seen in Linux kernel's drivers/xen/privcmd.c

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112975

David Malcolm <dmalcolm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #3 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Should be fixed by the above patch.