Am 03.06.2024 um 16:45 hat Fiona Ebner geschrieben: > Am 28.05.24 um 18:06 schrieb Kevin Wolf: > > Am 29.04.2024 um 16:19 hat Fiona Ebner geschrieben: > >> rather than the uint32_t for which the maximum is slightly more than 4 > >> seconds and larger values would overflow. The QAPI interface allows > >> specifying the number of seconds, so only values 0 to 4 are safe right > >> now, other values lead to a much lower timeout than a user expects. > >> > >> The block_copy() call where this is used already takes a uint64_t for > >> the timeout, so no change required there. > >> > >> Fixes: 6db7fd1ca9 ("block/copy-before-write: implement cbw-timeout option") > >> Reported-by: Friedrich Weber <f.we...@proxmox.com> > >> Signed-off-by: Fiona Ebner <f.eb...@proxmox.com> > > > > Thanks, applied to the block branch. > > > > But I don't think our job is done yet with this. Increasing the limit is > > good and useful, but even if it's now unlikely to hit with sane values, > > we should still catch integer overflows in cbw_open() and return an > > error on too big values instead of silently wrapping around. > > NANOSECONDS_PER_SECOND is 10^9 and the QAPI type for cbw-timeout is > uint32_t, so even with the maximum allowed value, there is no overflow. > Should I still add such a check?
You're right, I missed that cbw_timeout is uint32_t. So uint64_t will be always be enough to hold the result, and the calculation is also done in 64 bits because NANOSECONDS_PER_SECOND is long long. Then we don't need a check. Kevin