| Message ID | 73a89a42d82ec8b47358f25119b87063e4a6ea57.1721818306.git.mst@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | virtio-rng: block max-bytes=0 | expand |
On 24/07/2024 12:51, Michael S. Tsirkin wrote: > with max-bytes set to 0, quota is 0 and so device does not work. > block this to avoid user confusion > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > hw/virtio/virtio-rng.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c > index f74efffef7..7cf31da071 100644 > --- a/hw/virtio/virtio-rng.c > +++ b/hw/virtio/virtio-rng.c > @@ -184,8 +184,9 @@ static void virtio_rng_device_realize(DeviceState *dev, Error **errp) > > /* Workaround: Property parsing does not enforce unsigned integers, > * So this is a hack to reject such numbers. */ > - if (vrng->conf.max_bytes > INT64_MAX) { > - error_setg(errp, "'max-bytes' parameter must be non-negative, " > + if (vrng->conf.max_bytes == 0 || > + vrng->conf.max_bytes > INT64_MAX) { > + error_setg(errp, "'max-bytes' parameter must be positive, " > "and less than 2^63"); > return; > } Reviewed-by: Laurent Vivier <lvivier@redhat.com>
On Wed, 2024-07-24 at 06:51 -0400, Michael S. Tsirkin wrote: > with max-bytes set to 0, quota is 0 and so device does not work. > block this to avoid user confusion > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > hw/virtio/virtio-rng.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c > index f74efffef7..7cf31da071 100644 > --- a/hw/virtio/virtio-rng.c > +++ b/hw/virtio/virtio-rng.c > @@ -184,8 +184,9 @@ static void virtio_rng_device_realize(DeviceState > *dev, Error **errp) > > /* Workaround: Property parsing does not enforce unsigned > integers, > * So this is a hack to reject such numbers. */ > - if (vrng->conf.max_bytes > INT64_MAX) { > - error_setg(errp, "'max-bytes' parameter must be non- > negative, " > + if (vrng->conf.max_bytes == 0 || > + vrng->conf.max_bytes > INT64_MAX) { > + error_setg(errp, "'max-bytes' parameter must be positive, " > "and less than 2^63"); > return; > } Reviewed-by: Amit Shah <amit@kernel.org>
On 24/7/24 12:51, Michael S. Tsirkin wrote: > with max-bytes set to 0, quota is 0 and so device does not work. > block this to avoid user confusion > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > hw/virtio/virtio-rng.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
On 24/07/2024 12:51, Michael S. Tsirkin wrote: > with max-bytes set to 0, quota is 0 and so device does not work. > block this to avoid user confusion > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > hw/virtio/virtio-rng.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c > index f74efffef7..7cf31da071 100644 > --- a/hw/virtio/virtio-rng.c > +++ b/hw/virtio/virtio-rng.c > @@ -184,8 +184,9 @@ static void virtio_rng_device_realize(DeviceState *dev, Error **errp) > > /* Workaround: Property parsing does not enforce unsigned integers, > * So this is a hack to reject such numbers. */ > - if (vrng->conf.max_bytes > INT64_MAX) { > - error_setg(errp, "'max-bytes' parameter must be non-negative, " > + if (vrng->conf.max_bytes == 0 || > + vrng->conf.max_bytes > INT64_MAX) { > + error_setg(errp, "'max-bytes' parameter must be positive, " > "and less than 2^63"); > return; > } Michael, do you plan to send the PR yourself? Or perhaps it can go via trivial? Thanks, Laurent
On Tue, Jul 30, 2024 at 11:07:11AM +0200, Laurent Vivier wrote: > On 24/07/2024 12:51, Michael S. Tsirkin wrote: > > with max-bytes set to 0, quota is 0 and so device does not work. > > block this to avoid user confusion > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > --- > > hw/virtio/virtio-rng.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c > > index f74efffef7..7cf31da071 100644 > > --- a/hw/virtio/virtio-rng.c > > +++ b/hw/virtio/virtio-rng.c > > @@ -184,8 +184,9 @@ static void virtio_rng_device_realize(DeviceState *dev, Error **errp) > > /* Workaround: Property parsing does not enforce unsigned integers, > > * So this is a hack to reject such numbers. */ > > - if (vrng->conf.max_bytes > INT64_MAX) { > > - error_setg(errp, "'max-bytes' parameter must be non-negative, " > > + if (vrng->conf.max_bytes == 0 || > > + vrng->conf.max_bytes > INT64_MAX) { > > + error_setg(errp, "'max-bytes' parameter must be positive, " > > "and less than 2^63"); > > return; > > } > > Michael, do you plan to send the PR yourself? Or perhaps it can go via trivial? > > Thanks, > Laurent I will pick it up, I do not think this is appropriate for trivial though.
diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c index f74efffef7..7cf31da071 100644 --- a/hw/virtio/virtio-rng.c +++ b/hw/virtio/virtio-rng.c @@ -184,8 +184,9 @@ static void virtio_rng_device_realize(DeviceState *dev, Error **errp) /* Workaround: Property parsing does not enforce unsigned integers, * So this is a hack to reject such numbers. */ - if (vrng->conf.max_bytes > INT64_MAX) { - error_setg(errp, "'max-bytes' parameter must be non-negative, " + if (vrng->conf.max_bytes == 0 || + vrng->conf.max_bytes > INT64_MAX) { + error_setg(errp, "'max-bytes' parameter must be positive, " "and less than 2^63"); return; }
with max-bytes set to 0, quota is 0 and so device does not work. block this to avoid user confusion Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- hw/virtio/virtio-rng.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)