[v5,0/5] off-by-one and NULL pointer accesses detected by static analysis

Message

Liam Merwick Nov. 5, 2018, 9:38 p.m. UTC

Below are a number of fixes to some off-by-one, read outside array bounds, and
NULL pointer accesses detected by an internal Oracle static analysis tool (Parfait).
https://labs.oracle.com/pls/apex/f?p=labs:49:::::P49_PROJECT_ID:13

v1 -> v2
Based on feedback from Eric Blake:
patch2: reworded commit message to clarify issue
patch6: Reverted common qlist routines and added assert to qlist_dump instead
patch7: Fixed incorrect logic
patch8: Added QEMU_BUILD_BUG_ON to catch future іnstance at compile-time

v2 -> v3
Based on feedback from Eric Blake:
patch6: removed double space from commit message
patch8: removed unnecessary comment and updated QEMU_BUILD_BUG_ON to use ARRAY_SIZE
Added Eric's R-b to patches 6,7,8

v3 -> v4
Based on feedback from Max Reitz:
patch2: Added R-b from John Snow
patch3: fixed blk_get_attached_dev_id() instead of checking return value
patch4: switched to assert()
patch5: numerous changes based on feedback from Max
patch6: updated commit message
patch7: (was patch8): Added Max's R-b
patch8: (new): patch fixing NULL pointer dereference in kvm_arch_init_vcpu()

v4 -> v5
Based on further feedback from Max Reitz:
Dropped v4 patch1 (configure --disable-avx2) as Thomas Huth already pulled it. 
Dropped v4 patch6 (dump_qlist) as it was just an unnecessary assert
Dropped v4 patch8 'patch fixing NULL pointer dereference in kvm_arch_init_vcpu()'
  so as to limit this seies to block changes (will send in a separate series).
patch1: no change (v4 patch2)
patch2: Switched to using ?: in return (v4 patch3)
patch3: Added Max's R-b (v4 patch4)
patch4: couple of changes based on feedback from Max (v4 patch5)
patch5: no change (v4 patch7)

Liam Merwick (5):
  job: Fix off-by-one assert checks for JobSTT and JobVerbTable
  block: Null pointer dereference in blk_root_get_parent_desc()
  qemu-img: assert block_job_get() does not return NULL in img_commit()
  block: Fix potential Null pointer dereferences in vvfat.c
  qcow2: Read outside array bounds in qcow2_pre_write_overlap_check()

 block/block-backend.c  |  3 ++-
 block/qcow2-refcount.c | 18 ++++++++++--------
 block/vvfat.c          | 49 +++++++++++++++++++++++++++++++++----------------
 job.c                  |  4 ++--
 qemu-img.c             |  1 +
 5 files changed, 48 insertions(+), 27 deletions(-)

Comments

Max Reitz Nov. 11, 2018, 7:31 p.m. UTC | #1

On 05.11.18 22:38, Liam Merwick wrote:
> Below are a number of fixes to some off-by-one, read outside array bounds, and
> NULL pointer accesses detected by an internal Oracle static analysis tool (Parfait).
> https://labs.oracle.com/pls/apex/f?p=labs:49:::::P49_PROJECT_ID:13

I decided to just fix the issue I had in patch 4 (dropped the "if" block
that was not doing much, and fixed the DLOG() indentation) and applied
the patch to my block branch:

https://git.xanclic.moe/XanClic/qemu/commits/branch/block

Max