[Bug analyzer/112977] New: -Wanalyzer-tainted-offset false positive seen on Linux kernel's drivers/scsi/aacraid/aachba.c

[Bug analyzer/112977] New: -Wanalyzer-tainted-offset false positive seen on Linux kernel's drivers/scsi/aacraid/aachba.c

[dmalcolm at gcc dot gnu.org]

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

            Bug ID: 112977
           Summary: -Wanalyzer-tainted-offset false positive seen on Linux
                    kernel's drivers/scsi/aacraid/aachba.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: ---

drivers/scsi/aacraid/aachba.c: In function ‘force_delete_disk’:
drivers/scsi/aacraid/aachba.c:3307:36: warning: use of attacker-controlled
value as offset without upper-bounds checking [CWE-823]
[-Wanalyzer-tainted-offset]
 3307 |         fsa_dev_ptr[dd.cnum].valid = 0;
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
  ‘force_delete_disk’: events 1-7
    |
    | 3292 |         if (!fsa_dev_ptr)
    |      |            ^
    |      |            |
    |      |            (1) following ‘false’ branch (when ‘fsa_dev_ptr’ is
non-NULL)...
    |......
    | 3295 |         if (copy_from_user(&dd, arg, sizeof (struct
aac_delete_disk)))
    |      |         ~~ ~
    |      |         |  |
    |      |         |  (3) following ‘false’ branch (when ‘n == 0’)...
    |      |         (2) ...to here
    |......
    | 3298 |         if (dd.cnum >= dev->maximum_num_containers)
    |      |         ~~ ~
    |      |         |  |
    |      |         |  (5) following ‘false’ branch...
    |      |         (4) ...to here
    |......
    | 3303 |         fsa_dev_ptr[dd.cnum].deleted = 1;
    |      |         ~~~~~~~~~~~
    |      |         |
    |      |         (6) ...to here
    |......
    | 3307 |         fsa_dev_ptr[dd.cnum].valid = 0;
    |      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |                                    |
    |      |                                    (7) use of attacker-controlled
value as offset without upper-bounds checking
    |
drivers/scsi/aacraid/aachba.c: In function ‘delete_disk’:
drivers/scsi/aacraid/aachba.c:3335:49: warning: use of attacker-controlled
value as offset without upper-bounds checking [CWE-823]
[-Wanalyzer-tainted-offset]
 3335 |                 fsa_dev_ptr[dd.cnum].devname[0] = '\0';
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~
  ‘delete_disk’: events 1-9
    |
    | 3317 |         if (!fsa_dev_ptr)
    |      |            ^
    |      |            |
    |      |            (1) following ‘false’ branch (when ‘fsa_dev_ptr’ is
non-NULL)...
    |......
    | 3320 |         if (copy_from_user(&dd, arg, sizeof (struct
aac_delete_disk)))
    |      |         ~~ ~
    |      |         |  |
    |      |         |  (3) following ‘false’ branch (when ‘n == 0’)...
    |      |         (2) ...to here
    |......
    | 3323 |         if (dd.cnum >= dev->maximum_num_containers)
    |      |         ~~ ~
    |      |         |  |
    |      |         |  (5) following ‘false’ branch...
    |      |         (4) ...to here
    |......
    | 3328 |         if (fsa_dev_ptr[dd.cnum].locked)
    |      |         ~~ ~
    |      |         |  |
    |      |         |  (7) following ‘false’ branch...
    |      |         (6) ...to here
    |......
    | 3334 |                 fsa_dev_ptr[dd.cnum].valid = 0;
    |      |                 ~~~~~~~~~~~
    |      |                 |
    |      |                 (8) ...to here
    | 3335 |                 fsa_dev_ptr[dd.cnum].devname[0] = '\0';
    |      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |                                                 |
    |      |                                                 (9) use of
attacker-controlled value as offset without upper-bounds checking
    |

In both of these, dd.cnum is clearly checked, both at event (5)


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/112977] -Wanalyzer-tainted-offset false positive seen on Linux kernel's drivers/scsi/aacraid/aachba.c

[dmalcolm at gcc dot gnu.org]

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

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

[Bug analyzer/112977] -Wanalyzer-tainted-offset false positive seen on Linux kernel's drivers/scsi/aacraid/aachba.c

[cvs-commit at gcc dot gnu.org]

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

--- 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:e503f9aca9192654d83f141ae7865a3c9d90bf0d

commit r14-8391-ge503f9aca9192654d83f141ae7865a3c9d90bf0d
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Wed Jan 24 10:11:35 2024 -0500

    analyzer: fix taint false +ve due to overzealous state purging [PR112977]

    gcc/analyzer/ChangeLog:
            PR analyzer/112977
            * engine.cc (impl_region_model_context::on_liveness_change): Pass
            m_ext_state to sm_state_map::on_liveness_change.
            * program-state.cc (sm_state_map::on_svalue_leak): Guard removal
            of map entry based on can_purge_p.
            (sm_state_map::on_liveness_change): Add ext_state param.  Add
            workaround for bad interaction between state purging and
            alt-inherited sm-state.
            * program-state.h (sm_state_map::on_liveness_change): Add
            ext_state param.
            * sm-taint.cc
            (taint_state_machine::has_alt_get_inherited_state_p): New.
            (taint_state_machine::can_purge_p): Return false for "has_lb" and
            "has_ub".
            * sm.h (state_machine::has_alt_get_inherited_state_p): New vfunc.

    gcc/testsuite/ChangeLog:
            PR analyzer/112977
            * gcc.dg/plugin/plugin.exp: Add taint-pr112977.c.
            * gcc.dg/plugin/taint-pr112977.c: New test.

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

[Bug analyzer/112977] -Wanalyzer-tainted-offset false positive seen on Linux kernel's drivers/scsi/aacraid/aachba.c

[dmalcolm at gcc dot gnu.org]

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

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

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

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