diff options
author | Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> | 2023-10-05 17:03:26 +0300 |
---|---|---|
committer | Paolo Bonzini <pbonzini@redhat.com> | 2023-11-24 12:07:47 +0100 |
commit | adff55b520ef9ad2907a91409b152220c1ba8051 (patch) | |
tree | e12e99d09d7669d7620e0950e986f0c4ab5d4930 /system/physmem.c | |
parent | 6ef164188d005d7636f7ed8a1033cc4083498301 (diff) |
coverity: physmem: use simple assertions instead of modelling
Unfortunately Coverity doesn't follow the logic aroung "len" and "l"
variables in stacks finishing with flatview_{read,write}_continue() and
generate a lot of OVERRUN false-positives. When small buffer (2 or 4
bytes) is passed to mem read/write path, Coverity assumes the worst
case of sz=8 in stn_he_p()/ldn_he_p() (defined in
include/qemu/bswap.h), and reports buffer overrun.
To silence these false-positives we have model functions, which hide
real logic from Coverity.
However, it turned out that these new two assertions are enough to
quiet Coverity.
Assertions are better than hiding the logic, so let's drop the
modelling and move to assertions for memory r/w call stacks.
After patch, the sequence
cov-make-library --output-file /tmp/master.xmldb \
scripts/coverity-scan/model.c
cov-build --dir ~/covtmp/master make -j9
cov-analyze --user-model-file /tmp/master.xmldb \
--dir ~/covtmp/master --all --strip-path "$(pwd)
cov-format-errors --dir ~/covtmp/master \
--html-output ~/covtmp/master_html_report
Generate for me the same big set of CIDs excepept for 6 disappeared (so
it becomes even better).
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Acked-by: David Hildenbrand <david@redhat.com>
Message-ID: <20231005140326.332830-1-vsementsov@yandex-team.ru>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Diffstat (limited to 'system/physmem.c')
-rw-r--r-- | system/physmem.c | 22 |
1 files changed, 22 insertions, 0 deletions
diff --git a/system/physmem.c b/system/physmem.c index fc2b0fee01..a63853a7bc 100644 --- a/system/physmem.c +++ b/system/physmem.c @@ -2699,6 +2699,17 @@ static MemTxResult flatview_write_continue(FlatView *fv, hwaddr addr, l = memory_access_size(mr, l, addr1); /* XXX: could force current_cpu to NULL to avoid potential bugs */ + + /* + * Assure Coverity (and ourselves) that we are not going to OVERRUN + * the buffer by following ldn_he_p(). + */ +#ifdef QEMU_STATIC_ANALYSIS + assert((l == 1 && len >= 1) || + (l == 2 && len >= 2) || + (l == 4 && len >= 4) || + (l == 8 && len >= 8)); +#endif val = ldn_he_p(buf, l); result |= memory_region_dispatch_write(mr, addr1, val, size_memop(l), attrs); @@ -2769,6 +2780,17 @@ MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr, l = memory_access_size(mr, l, addr1); result |= memory_region_dispatch_read(mr, addr1, &val, size_memop(l), attrs); + + /* + * Assure Coverity (and ourselves) that we are not going to OVERRUN + * the buffer by following stn_he_p(). + */ +#ifdef QEMU_STATIC_ANALYSIS + assert((l == 1 && len >= 1) || + (l == 2 && len >= 2) || + (l == 4 && len >= 4) || + (l == 8 && len >= 8)); +#endif stn_he_p(buf, l, val); } else { /* RAM case */ |