On Tue, 23 Jul 2024 at 16:44, Philippe Mathieu-Daudé <[email protected]> wrote: > > On 23/7/24 17:09, Peter Maydell wrote: > > aio_context_set_thread_pool_params() takes two int64_t arguments to > > set the minimum and maximum number of threads in the pool. We do > > some bounds checking on these, but we don't catch the case where the > > inputs are negative. This means that later in the function when we > > assign these inputs to the AioContext::thread_pool_min and > > ::thread_pool_max fields, which are of type int, the values might > > overflow the smaller type. > > > > A negative number of threads is meaningless, so make > > aio_context_set_thread_pool_params() return an error if either min or > > max are negative. > > > > Resolves: Coverity CID 1547605 > > Signed-off-by: Peter Maydell <[email protected]> > > --- > > util/async.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/util/async.c b/util/async.c > > index 0467890052a..3e3e4fc7126 100644 > > --- a/util/async.c > > +++ b/util/async.c > > @@ -746,7 +746,7 @@ void aio_context_set_thread_pool_params(AioContext > > *ctx, int64_t min, > > int64_t max, Error **errp) > > { > > > > - if (min > max || !max || min > INT_MAX || max > INT_MAX) { > > + if (min > max || max <= 0 || min < 0 || min > INT_MAX || max > > > INT_MAX) { > > error_setg(errp, "bad thread-pool-min/thread-pool-max values"); > > return; > > } > > Reviewed-by: Philippe Mathieu-Daudé <[email protected]> > > I don't get the point of using signed min/max here...
I think this is because those values may originate in a QAPI command structure (EventLoopBaseProperties), where they are defined as "int" rather than a specifically unsigned type. So we carry them around as int64_t until they get to here, where we do the validation of whether they're sensible or not. thanks -- PMM
