OpenE2K/qemu-e2k
13
4
Fork 0
Code Issues 4 Pull Requests Projects 2 Releases Activity

coverity: physmem: use simple assertions instead of modelling

Browse Source 
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>
This commit is contained in:
Vladimir Sementsov-Ogievskiy 2023-10-05 17:03:26 +03:00 committed by Paolo Bonzini
parent 6ef164188d
commit adff55b520
2 changed files with 22 additions and 88 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

88
scripts/coverity-scan/model.c
View File

@ -42,94 +42,6 @@ typedef _Bool bool;
typedef struct va_list_str *va_list;
/* exec.c */
typedef struct AddressSpace AddressSpace;
typedef struct MemoryRegionCache MemoryRegionCache;
typedef uint64_t hwaddr;
typedef uint32_t MemTxResult;
typedef struct MemTxAttrs {} MemTxAttrs;
static void __bufwrite(uint8_t *buf, ssize_t len)
{
int first, last;
__coverity_negative_sink__(len);
if (len == 0) return;
buf[0] = first;
buf[len-1] = last;
__coverity_writeall__(buf);
}
static void __bufread(uint8_t *buf, ssize_t len)
{
__coverity_negative_sink__(len);
if (len == 0) return;
int first = buf[0];
int last = buf[len-1];
}
MemTxResult address_space_read_cached(MemoryRegionCache *cache, hwaddr addr,
MemTxAttrs attrs,
void *buf, int len)
{
MemTxResult result;
// TODO: investigate impact of treating reads as producing
// tainted data, with __coverity_tainted_data_argument__(buf).
__bufwrite(buf, len);
return result;
}
MemTxResult address_space_write_cached(MemoryRegionCache *cache, hwaddr addr,
MemTxAttrs attrs,
const void *buf, int len)
{
MemTxResult result;
__bufread(buf, len);
return result;
}
MemTxResult address_space_rw_cached(MemoryRegionCache *cache, hwaddr addr,
MemTxAttrs attrs,
void *buf, int len, bool is_write)
{
if (is_write) {
return address_space_write_cached(cache, addr, attrs, buf, len);
} else {
return address_space_read_cached(cache, addr, attrs, buf, len);
}
}
MemTxResult address_space_read(AddressSpace *as, hwaddr addr,
MemTxAttrs attrs,
void *buf, int len)
{
MemTxResult result;
// TODO: investigate impact of treating reads as producing
// tainted data, with __coverity_tainted_data_argument__(buf).
__bufwrite(buf, len);
return result;
}
MemTxResult address_space_write(AddressSpace *as, hwaddr addr,
MemTxAttrs attrs,
const void *buf, int len)
{
MemTxResult result;
__bufread(buf, len);
return result;
}
MemTxResult address_space_rw(AddressSpace *as, hwaddr addr,
MemTxAttrs attrs,
void *buf, int len, bool is_write)
{
if (is_write) {
return address_space_write(as, addr, attrs, buf, len);
} else {
return address_space_read(as, addr, attrs, buf, len);
}
}
/* Tainting */
typedef struct {} name2keysym_t;

22
system/physmem.c
View File

@ -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 */