[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PULL 07/21] cutils: Fix wraparound parsing in qemu_strtoui
From: |
Eric Blake |
Subject: |
Re: [PULL 07/21] cutils: Fix wraparound parsing in qemu_strtoui |
Date: |
Mon, 5 Jun 2023 08:32:55 -0500 |
User-agent: |
NeoMutt/20230517 |
On Sat, Jun 03, 2023 at 11:17:27AM +0300, Michael Tokarev wrote:
> 02.06.2023 01:02, Eric Blake пишет:
> > While we were matching 32-bit strtol in qemu_strtoi, our use of a
> > 64-bit parse was leaking through for some inaccurate answers in
> > qemu_strtoui in comparison to a 32-bit strtoul (see the unit test for
> > examples). The comment for that function even described what we have
> > to do for a correct parse, but didn't implement it correctly: since
> > strtoull checks for overflow against the wrong values and then
> > negates, we have to temporarily undo negation before checking for
> > overflow against our desired value.
> >
> > Our int wrappers would be a lot easier to write if libc had a
> > guaranteed 32-bit parser even on platforms with 64-bit long.
> >
> > Whether we parse C2x binary strings like "0b1000" is currently up to
> > what libc does; our unit tests intentionally don't cover that at the
> > moment, though.
> >
> > Fixes: 473a2a331e ("cutils: add qemu_strtoi & qemu_strtoui parsers for
> > int/unsigned int types", v2.12.0)
> > Signed-off-by: Eric Blake <eblake@redhat.com>
> > CC: qemu-stable@nongnu.org
>
> Trying to pick this one up for -stable. The implementation changes are good.
> But the testsuite changes are.. difficult. The thing is that testsuite
> changes
> (here and in the other -stable patch) applies on top of previous changes in
> there (in the same series), which, in turn, requires other previous code
> changes
> in the implementation to succeed.
Yeah, I did wonder if it is worth even trying to get this for stable.
The series is inter-tangled enough that it feels like an
all-or-nothing approach may be easiest - but all means 19 patches and
hundreds of lines of testsuite additions. My other argument when
first posting this to qemu-stable was to just declare that these have
been broken long enough that it is not a recent regression, and users
are unlikely to be supplying command-line or QMP strings to tickle
these bugs, so not backporting may be okay.
>
> For example, this patch changes test_qemu_strtoui_overflow() which was
> introduced
> in previous patch "Test more integer corner cases" from this series and
> further
> modified in "Test integral qemu_strto* value on failures" one. Picking them
> result
> in testsuite failing due to missing previous code changes.
>
> I tried to drop just the testsuite changes, but the result is that the
> testsuite
> fails with fixed code :)
>
> It's quite fun situation actually, like it fails no matter what you do, one
> way
> or another.
>
> I'll try to find the most stright-forward way from here. Good stuff.
Let me know how I can help in deciding what, if any, is worth backporting.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
- [PULL 00/21] NBD and miscellaneous patches for 2023-06-01, Eric Blake, 2023/06/01
- [PULL 02/21] qcow2: Explicit mention of padding bytes, Eric Blake, 2023/06/01
- [PULL 05/21] test-cutils: Test integral qemu_strto* value on failures, Eric Blake, 2023/06/01
- [PULL 01/21] iotests: Fix test 104 under NBD, Eric Blake, 2023/06/01
- [PULL 08/21] cutils: Document differences between parse_uint and qemu_strtou64, Eric Blake, 2023/06/01
- [PULL 10/21] cutils: Allow NULL endptr in parse_uint(), Eric Blake, 2023/06/01
- [PULL 11/21] test-cutils: Add coverage of qemu_strtod, Eric Blake, 2023/06/01
- [PULL 07/21] cutils: Fix wraparound parsing in qemu_strtoui, Eric Blake, 2023/06/01
- [PULL 09/21] cutils: Adjust signature of parse_uint[_full], Eric Blake, 2023/06/01
- [PULL 18/21] cutils: Set value in all integral qemu_strto* error paths, Eric Blake, 2023/06/01
- [PULL 16/21] test-cutils: Add more coverage to qemu_strtosz, Eric Blake, 2023/06/01
- [PULL 21/21] cutils: Improve qemu_strtosz handling of fractions, Eric Blake, 2023/06/01
- [PULL 04/21] test-cutils: Use g_assert_cmpuint where appropriate, Eric Blake, 2023/06/01
- [PULL 15/21] numa: Check for qemu_strtosz_MiB error, Eric Blake, 2023/06/01
- [PULL 20/21] cutils: Improve qemu_strtod* error paths, Eric Blake, 2023/06/01
- [PULL 17/21] cutils: Set value in all qemu_strtosz* error paths, Eric Blake, 2023/06/01