[v2,00/19] Fix qemu_strtosz() read-out-of-bounds

Message

Eric Blake May 12, 2023, 2:10 a.m. UTC

v1 was here:
https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg01988.html

since then:
- make parse_uint easier to use, then use it in qemu_strtosz
- add even more unit tests
- fix a bug in qemu_strtoui
- avoid dereferencing randome memory during unit tests [Hanna]
- other cleanups as I found them
- compress the strtosz unit tests (the major cause of the large
  interdiff statistics)

backport-diff looks like:

001/19:[----] [--] 'test-cutils: Avoid g_assert in unit tests'
002/19:[----] [--] 'test-cutils: Use g_assert_cmpuint where appropriate'
003/19:[----] [--] 'test-cutils: Test integral qemu_strto* value on failures'
004/19:[down] 'test-cutils: Test more integer corner cases'
005/19:[down] 'cutils: Fix wraparound parsing in qemu_strtoui'
006/19:[down] 'cutils: Document differences between parse_uint and qemu_strtou64'
007/19:[down] 'cutils: Adjust signature of parse_uint[_full]'
008/19:[down] 'cutils: Allow NULL endptr in parse_uint()'
009/19:[0147] [FC] 'test-cutils: Add coverage of qemu_strtod'
010/19:[----] [--] 'test-cutils: Prepare for upcoming semantic change in qemu_strtosz'
011/19:[down] 'test-cutils: Refactor qemu_strtosz tests for less boilerplate'
012/19:[down] 'cutils: Allow NULL str in qemu_strtosz'
013/19:[----] [--] 'numa: Check for qemu_strtosz_MiB error'
014/19:[down] 'test-cutils: Add more coverage to qemu_strtosz11;rgb:1e1e/1e1e/1e1e'
015/19:[0178] [FC] 'cutils: Set value in all qemu_strtosz* error paths'
016/19:[----] [--] 'cutils: Set value in all integral qemu_strto* error paths'
017/19:[down] 'cutils: Use parse_uint in qemu_strtosz for negative rejection'
018/19:[0018] [FC] 'cutils: Improve qemu_strtod* error paths'
019/19:[0107] [FC] 'cutils: Improve qemu_strtosz handling of fractions'


Eric Blake (19):
  test-cutils: Avoid g_assert in unit tests
  test-cutils: Use g_assert_cmpuint where appropriate
  test-cutils: Test integral qemu_strto* value on failures
  test-cutils: Test more integer corner cases
  cutils: Fix wraparound parsing in qemu_strtoui
  cutils: Document differences between parse_uint and qemu_strtou64
  cutils: Adjust signature of parse_uint[_full]
  cutils: Allow NULL endptr in parse_uint()
  test-cutils: Add coverage of qemu_strtod
  test-cutils: Prepare for upcoming semantic change in qemu_strtosz
  test-cutils: Refactor qemu_strtosz tests for less boilerplate
  cutils: Allow NULL str in qemu_strtosz
  numa: Check for qemu_strtosz_MiB error
  test-cutils: Add more coverage to qemu_strtosz11;rgb:1e1e/1e1e/1e1e
  cutils: Set value in all qemu_strtosz* error paths
  cutils: Set value in all integral qemu_strto* error paths
  cutils: Use parse_uint in qemu_strtosz for negative rejection
  cutils: Improve qemu_strtod* error paths
  cutils: Improve qemu_strtosz handling of fractions

 include/qemu/cutils.h         |    5 +-
 audio/audio_legacy.c          |    4 +-
 block/gluster.c               |    4 +-
 block/nfs.c                   |    4 +-
 blockdev.c                    |    4 +-
 contrib/ivshmem-server/main.c |    4 +-
 hw/core/numa.c                |   11 +-
 qapi/opts-visitor.c           |   10 +-
 tests/unit/test-cutils.c      | 2340 ++++++++++++++++++++++++---------
 ui/vnc.c                      |    4 +-
 util/cutils.c                 |  251 ++--
 util/guest-random.c           |    4 +-
 util/qemu-sockets.c           |   10 +-
 13 files changed, 1891 insertions(+), 764 deletions(-)


base-commit: 278238505d28d292927bff7683f39fb4fbca7fd1

Comments

Eric Blake May 12, 2023, 12:24 p.m. UTC | #1

On Thu, May 11, 2023 at 09:10:14PM -0500, Eric Blake wrote:
> v1 was here:
> https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg01988.html
> 
> since then:
> - make parse_uint easier to use, then use it in qemu_strtosz
> - add even more unit tests
> - fix a bug in qemu_strtoui
> - avoid dereferencing randome memory during unit tests [Hanna]
> - other cleanups as I found them
> - compress the strtosz unit tests (the major cause of the large
>   interdiff statistics)
> 
> backport-diff looks like:
> 
> 001/19:[----] [--] 'test-cutils: Avoid g_assert in unit tests'
> 002/19:[----] [--] 'test-cutils: Use g_assert_cmpuint where appropriate'
> 003/19:[----] [--] 'test-cutils: Test integral qemu_strto* value on failures'
> 004/19:[down] 'test-cutils: Test more integer corner cases'
> 005/19:[down] 'cutils: Fix wraparound parsing in qemu_strtoui'
> 006/19:[down] 'cutils: Document differences between parse_uint and qemu_strtou64'
> 007/19:[down] 'cutils: Adjust signature of parse_uint[_full]'
> 008/19:[down] 'cutils: Allow NULL endptr in parse_uint()'
> 009/19:[0147] [FC] 'test-cutils: Add coverage of qemu_strtod'
> 010/19:[----] [--] 'test-cutils: Prepare for upcoming semantic change in qemu_strtosz'
> 011/19:[down] 'test-cutils: Refactor qemu_strtosz tests for less boilerplate'
> 012/19:[down] 'cutils: Allow NULL str in qemu_strtosz'
> 013/19:[----] [--] 'numa: Check for qemu_strtosz_MiB error'
> 014/19:[down] 'test-cutils: Add more coverage to qemu_strtosz11;rgb:1e1e/1e1e/1e1e'

Not sure how I managed to corrupt that subject line while rebasing
(looks like a read race on /dev/tty where my editor intercepted bytes
intended to go to the shell's terminal); a corrected version is now
available at:

git fetch https://repo.or.cz/qemu/ericb.git strtosz
https://repo.or.cz/qemu/ericb.git/tree/refs/heads/strtosz

and with that fixed, this line changes to:

014/18:[0335] [FC] 'test-cutils: Add more coverage to qemu_strtosz'

> 015/19:[0178] [FC] 'cutils: Set value in all qemu_strtosz* error paths'
> 016/19:[----] [--] 'cutils: Set value in all integral qemu_strto* error paths'
> 017/19:[down] 'cutils: Use parse_uint in qemu_strtosz for negative rejection'
> 018/19:[0018] [FC] 'cutils: Improve qemu_strtod* error paths'
> 019/19:[0107] [FC] 'cutils: Improve qemu_strtosz handling of fractions'
>