[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PULL v2 03/16] block/block-backend: add block layer APIs resembling
From: |
Stefan Hajnoczi |
Subject: |
Re: [PULL v2 03/16] block/block-backend: add block layer APIs resembling Linux ZonedBlockDevice ioctls |
Date: |
Tue, 7 May 2024 11:09:19 -0400 |
On Fri, May 03, 2024 at 01:33:51PM +0100, Peter Maydell wrote:
> On Mon, 15 May 2023 at 17:07, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >
> > From: Sam Li <faithilikerun@gmail.com>
> >
> > Add zoned device option to host_device BlockDriver. It will be presented
> > only
> > for zoned host block devices. By adding zone management operations to the
> > host_block_device BlockDriver, users can use the new block layer APIs
> > including Report Zone and four zone management operations
> > (open, close, finish, reset, reset_all).
> >
> > Qemu-io uses the new APIs to perform zoned storage commands of the device:
> > zone_report(zrp), zone_open(zo), zone_close(zc), zone_reset(zrs),
> > zone_finish(zf).
> >
> > For example, to test zone_report, use following command:
> > $ ./build/qemu-io --image-opts -n driver=host_device, filename=/dev/nullb0
> > -c "zrp offset nr_zones"
>
> Hi; Coverity points out an issue in this commit (CID 1544771):
>
> > +static int zone_report_f(BlockBackend *blk, int argc, char **argv)
> > +{
> > + int ret;
> > + int64_t offset;
> > + unsigned int nr_zones;
> > +
> > + ++optind;
> > + offset = cvtnum(argv[optind]);
> > + ++optind;
> > + nr_zones = cvtnum(argv[optind]);
>
> cvtnum() can fail and return a negative value on error
> (e.g. if the number in the string is out of range),
> but we are not checking for that. Instead we stuff
> the value into an 'unsigned int' and then pass that to
> g_new(), which will result in our trying to allocate a large
> amount of memory.
>
> Here, and also in the other functions below that use cvtnum(),
> I think we should follow the pattern for use of that function
> that is used in the pre-existing code in this function:
>
> int64_t foo; /* NB: not an unsigned or some smaller type */
>
> foo = cvtnum(arg)
> if (foo < 0) {
> print_cvtnum_err(foo, arg);
> return foo; /* or otherwise handle returning an error upward */
> }
>
> It looks like all the uses of cvtnum in this patch should be
> adjusted to handle errors.
Thanks for letting me know. I will send a patch.
Stefan
signature.asc
Description: PGP signature