| Message ID | 20211005161157.282396-2-sgarzare@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block: avoid integer overflow of `max-workers` and assert `max_busy_tasks` | expand |
10/5/21 19:11, Stefano Garzarella wrote: > QAPI generates `struct BackupPerf` where `max-workers` value is stored > in an `int64_t` variable. > But block_copy_async(), and the underlying code, uses an `int` parameter. > > At the end that variable is used to initialize `max_busy_tasks` in > block/aio_task.c causing the following assertion failure if a value > greater than INT_MAX(2147483647) is used: > > ../block/aio_task.c:63: aio_task_pool_wait_one: Assertion `pool->busy_tasks > 0' failed. > > Let's check that `max-workers` doesn't exceed INT_MAX and print an > error in that case. > > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2009310 I glad to see that someone experiments with my experimental API :) > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > block/backup.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/block/backup.c b/block/backup.c > index 687d2882bc..8b072db5d9 100644 > --- a/block/backup.c > +++ b/block/backup.c > @@ -407,8 +407,8 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs, > return NULL; > } > > - if (perf->max_workers < 1) { > - error_setg(errp, "max-workers must be greater than zero"); > + if (perf->max_workers < 1 || perf->max_workers > INT_MAX) { > + error_setg(errp, "max-workers must be between 1 and %d", INT_MAX); > return NULL; > } > >
diff --git a/block/backup.c b/block/backup.c index 687d2882bc..8b072db5d9 100644 --- a/block/backup.c +++ b/block/backup.c @@ -407,8 +407,8 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs, return NULL; } - if (perf->max_workers < 1) { - error_setg(errp, "max-workers must be greater than zero"); + if (perf->max_workers < 1 || perf->max_workers > INT_MAX) { + error_setg(errp, "max-workers must be between 1 and %d", INT_MAX); return NULL; }
QAPI generates `struct BackupPerf` where `max-workers` value is stored in an `int64_t` variable. But block_copy_async(), and the underlying code, uses an `int` parameter. At the end that variable is used to initialize `max_busy_tasks` in block/aio_task.c causing the following assertion failure if a value greater than INT_MAX(2147483647) is used: ../block/aio_task.c:63: aio_task_pool_wait_one: Assertion `pool->busy_tasks > 0' failed. Let's check that `max-workers` doesn't exceed INT_MAX and print an error in that case. Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2009310 Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> --- block/backup.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)