[Bug analyzer/112927] New: -Wanalyzer-tainted-size false positive seen in Linux kernel's drivers/char/ipmi/ipmi_devintf.c

[Bug analyzer/112927] New: -Wanalyzer-tainted-size false positive seen in Linux kernel's drivers/char/ipmi/ipmi_devintf.c

[dmalcolm at gcc dot gnu.org]

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

            Bug ID: 112927
           Summary: -Wanalyzer-tainted-size false positive seen in Linux
                    kernel's drivers/char/ipmi/ipmi_devintf.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: ---

Created attachment 56837
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=56837&action=edit
Reduced reproducer

With the kernel plugin, this test erroenously reports:

In function 'call_copy_from_user',
    inlined from 'handle_send_req' at
gcc.dg/plugin/taint-drivers-char-ipmi-ipmi_devintf.c:35:7:
gcc.dg/plugin/taint-drivers-char-ipmi-ipmi_devintf.c:19:7: warning: use of
attacker-controlled value as size without upper-bounds checking [CWE-129]
[-Wanalyzer-tainted-size]
   19 |   n = copy_from_user(to, from, n); /* { dg-bogus "use of
attacker-controlled value as size without upper-bounds checking" } */
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~
  'ipmi_ioctl': events 1-4
    |
    |   41 | ipmi_ioctl(void* arg)
    |      | ^~~~~~~~~~
    |      | |
    |      | (1) entry to 'ipmi_ioctl'
    |......
    |   44 |   if (call_copy_from_user(&msg, arg, sizeof(msg))) {
    |      |      ~
    |      |      |
    |      |      (2) following 'false' branch (when 'n == 0')...
    |......
    |   48 |   return handle_send_req(&msg);
    |      |          ~~~~~~~~~~~~~~~~~~~~~
    |      |          |
    |      |          (3) ...to here
    |      |          (4) calling 'handle_send_req' from 'ipmi_ioctl'
    |
    +--> 'handle_send_req': events 5-8
           |
           |   29 | handle_send_req(struct ipmi_msg* msg)
           |      | ^~~~~~~~~~~~~~~
           |      | |
           |      | (5) entry to 'handle_send_req'
           |......
           |   32 |   if (msg->data_len > 272) {
           |      |      ~
           |      |      |
           |      |      (6) following 'false' branch...
           |......
           |   35 |   if (call_copy_from_user(buf, msg->data, msg->data_len)) {
           |      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           |      |       |
           |      |       (7) ...to here
           |      |       (8) inlined call to 'call_copy_from_user' from
'handle_send_req'
           |
           +--> 'call_copy_from_user': event 9
                  |
                  |   19 |   n = copy_from_user(to, from, n); /* { dg-bogus
"use of attacker-controlled value as size without upper-bounds checking" } */
                  |      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~
                  |      |       |
                  |      |       (9) use of attacker-controlled value as size
without upper-bounds checking
                  |


despite the value being sanitized at event (6).


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/112927] -Wanalyzer-tainted-size false positive seen in Linux kernel's drivers/char/ipmi/ipmi_devintf.c

[cvs-commit at gcc dot gnu.org]

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

--- Comment #1 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:b6e537571c21d8f0bc276d7afa156d6d4a54a1c9

commit r14-8390-gb6e537571c21d8f0bc276d7afa156d6d4a54a1c9
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Wed Jan 24 10:11:09 2024 -0500

    analyzer kernel plugin: implement __check_object_size [PR112927]

    PR analyzer/112927 reports a false positive from -Wanalyzer-tainted-size
    seen on the Linux kernel's drivers/char/ipmi/ipmi_devintf.c with the
    analyzer kernel plugin.

    The issue is that in:

    (A):
      if (msg->data_len > 272) {
        return -90;
      }

    (B):
      n = msg->data_len;
      __check_object_size(to, n);
      n = copy_from_user(to, from, n);

    the analyzer is treating __check_object_size as having arbitrary side
    effects, and, in particular could modify msg->data_len.  Hence the
    sanitization that occurs at (A) above is treated as being for a
    different value than the size obtained at (B), hence the bogus warning
    at the call to copy_from_user.

    Fixed by extending the analyzer kernel plugin to "teach" it that
    __check_object_size has no side effects.

    gcc/testsuite/ChangeLog:
            PR analyzer/112927
            * gcc.dg/plugin/analyzer_kernel_plugin.c
            (class known_function___check_object_size): New.
            (kernel_analyzer_init_cb): Register it.
            * gcc.dg/plugin/plugin.exp: Add taint-pr112927.c.
            * gcc.dg/plugin/taint-pr112927.c: New test.

    Signed-off-by: David Malcolm <dmalcolm@redhat.com>

[Bug analyzer/112927] -Wanalyzer-tainted-size false positive seen in Linux kernel's drivers/char/ipmi/ipmi_devintf.c

[dmalcolm at gcc dot gnu.org]

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

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

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

--- Comment #2 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Should be fixed on trunk for GCC 14 by the above patch; marking as resolved.