Am 12.03.2026 um 17:19 hat Peter Maydell geschrieben: > On Thu, 12 Mar 2026 at 16:13, Kevin Wolf <[email protected]> wrote: > > > > Am 12.03.2026 um 10:41 hat Peter Maydell geschrieben: > > > On Tue, 10 Mar 2026 at 16:30, Kevin Wolf <[email protected]> wrote: > > > > > > > > From: Peter Lieven <[email protected]> > > > > > > > > libnfs v6 added a new api structure for read and write requests. > > > > > > > > This effectively also adds zero copy read support for cases where > > > > the qiov coming from the block layer has only one vector. > > > > > > > > The .brdv_refresh_limits implementation is needed because libnfs v6 > > > > silently dropped support for splitting large read/write request into > > > > chunks. > > > > > > > > Signed-off-by: Ronnie Sahlberg <[email protected]> > > > > Signed-off-by: Peter Lieven <[email protected]> > > > > Message-ID: <[email protected]> > > > > Reviewed-by: Kevin Wolf <[email protected]> > > > > Signed-off-by: Kevin Wolf <[email protected]> > > > > > > > > > Hi; Coverity reports a potential issue with this code > > > (CID 1645631): > > > > > > > @@ -266,13 +270,36 @@ static int coroutine_fn > > > > nfs_co_preadv(BlockDriverState *bs, int64_t offset, > > > > { > > > > NFSClient *client = bs->opaque; > > > > NFSRPC task; > > > > + char *buf = NULL; > > > > + bool my_buffer = false; > > > > > > > > nfs_co_init_task(bs, &task); > > > > - task.iov = iov; > > > > + > > > > +#ifdef LIBNFS_API_V2 > > > > + if (iov->niov != 1) { > > > > + buf = g_try_malloc(bytes); > > > > + if (bytes && buf == NULL) { > > > > + return -ENOMEM; > > > > + } > > > > + my_buffer = true; > > > > > > Here we have code that takes the "read zero bytes" case, and > > > still tries to malloc a 0-length buffer (which is of dubious > > > portability). Then it will continue... > > > > g_try_malloc() always returns NULL for allocating 0 bytes, so I don't > > think portability is a problem here. > > Ah, so it does. The glib docs document this for g_malloc() but > don't mention it for g_try_malloc(), so I missed it. > > > > > + if (my_buffer) { > > > > + if (task.ret > 0) { > > > > + qemu_iovec_from_buf(iov, 0, buf, task.ret); > > > > > > ...and down here we use 'buf', but Coverity thinks it might be NULL > > > because we only returned -ENOMEM above for a NULL buffer if bytes == 0. > > > So we might be here with bytes == 0 and buf == NULL. > > > > Yes, buf might be NULL, but copying 0 bytes from NULL isn't a problem > > because you don't actually dereference the pointer then. > > > > I think the part that Coverity doesn't understand is probably that > > task.ret is limited to bytes (i.e. 0 in this case). > > Mmm. Let me know if you'd prefer me to mark this issue in > Coverity as a false positive rather than changing the code.
I don't really mind either way. If someone posts a patch, I'll apply it (though not sure if that would be only for 11.1 at this point). Kevin
