| Message ID | 161035859647.1221144.4691749806675653934.stgit@bahia.lan (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | 9pfs/proxy: Check return value of proxy_marshal() | expand |
On Montag, 11. Januar 2021 10:49:56 CET Greg Kurz wrote: > This should always successfully write exactly two 32-bit integers. > Make it clear with an assert(), like v9fs_receive_status() and > v9fs_receive_response() already do when unmarshalling the same > header. > > Fixes: Coverity CID 1438968 > Signed-off-by: Greg Kurz <groug@kaod.org> Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com> What's your workload Greg, are you able to push this through your queue? It's time that I signup for coverity. I'm doing that now before I forget it again. > --- > hw/9pfs/9p-proxy.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c > index 6f598a0f111c..4aa4e0a3baa0 100644 > --- a/hw/9pfs/9p-proxy.c > +++ b/hw/9pfs/9p-proxy.c > @@ -537,7 +537,8 @@ static int v9fs_request(V9fsProxy *proxy, int type, void > *response, ...) } > > /* marshal the header details */ > - proxy_marshal(iovec, 0, "dd", header.type, header.size); > + retval = proxy_marshal(iovec, 0, "dd", header.type, header.size); > + assert(retval == 4 * 2); > header.size += PROXY_HDR_SZ; > > retval = qemu_write_full(proxy->sockfd, iovec->iov_base, header.size); Best regards, Christian Schoenebeck
On Mon, 11 Jan 2021 14:15:17 +0100 Christian Schoenebeck <qemu_oss@crudebyte.com> wrote: > On Montag, 11. Januar 2021 10:49:56 CET Greg Kurz wrote: > > This should always successfully write exactly two 32-bit integers. > > Make it clear with an assert(), like v9fs_receive_status() and > > v9fs_receive_response() already do when unmarshalling the same > > header. > > > > Fixes: Coverity CID 1438968 > > Signed-off-by: Greg Kurz <groug@kaod.org> > > Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com> > > What's your workload Greg, are you able to push this through your queue? > I'll take care of the security issue first, likely later today or tomorrow. It is generally recommended to have separate PRs for CVEs, in order to ease the backport effort of downstream vendors. No hurry for this patch though. It isn't even a bug fix : we really can't get an error at this point since previous calls to proxy_marshal() in this function obviously succeeded at writing stuff at higher offsets... So this is really a cosmetic only change to make Coverity happy. I might be able to send a PR with this next week or the week after I guess. > It's time that I signup for coverity. I'm doing that now before I forget it > again. > > > --- > > hw/9pfs/9p-proxy.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c > > index 6f598a0f111c..4aa4e0a3baa0 100644 > > --- a/hw/9pfs/9p-proxy.c > > +++ b/hw/9pfs/9p-proxy.c > > @@ -537,7 +537,8 @@ static int v9fs_request(V9fsProxy *proxy, int type, void > > *response, ...) } > > > > /* marshal the header details */ > > - proxy_marshal(iovec, 0, "dd", header.type, header.size); > > + retval = proxy_marshal(iovec, 0, "dd", header.type, header.size); > > + assert(retval == 4 * 2); > > header.size += PROXY_HDR_SZ; > > > > retval = qemu_write_full(proxy->sockfd, iovec->iov_base, header.size); > > Best regards, > Christian Schoenebeck > >
On Mon, 11 Jan 2021 10:49:56 +0100 Greg Kurz <groug@kaod.org> wrote: > This should always successfully write exactly two 32-bit integers. > Make it clear with an assert(), like v9fs_receive_status() and > v9fs_receive_response() already do when unmarshalling the same > header. > > Fixes: Coverity CID 1438968 > Signed-off-by: Greg Kurz <groug@kaod.org> > --- Applied to https://gitlab.com/gkurz/qemu/-/tree/9p-next > hw/9pfs/9p-proxy.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c > index 6f598a0f111c..4aa4e0a3baa0 100644 > --- a/hw/9pfs/9p-proxy.c > +++ b/hw/9pfs/9p-proxy.c > @@ -537,7 +537,8 @@ static int v9fs_request(V9fsProxy *proxy, int type, void *response, ...) > } > > /* marshal the header details */ > - proxy_marshal(iovec, 0, "dd", header.type, header.size); > + retval = proxy_marshal(iovec, 0, "dd", header.type, header.size); > + assert(retval == 4 * 2); > header.size += PROXY_HDR_SZ; > > retval = qemu_write_full(proxy->sockfd, iovec->iov_base, header.size); > >
diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c index 6f598a0f111c..4aa4e0a3baa0 100644 --- a/hw/9pfs/9p-proxy.c +++ b/hw/9pfs/9p-proxy.c @@ -537,7 +537,8 @@ static int v9fs_request(V9fsProxy *proxy, int type, void *response, ...) } /* marshal the header details */ - proxy_marshal(iovec, 0, "dd", header.type, header.size); + retval = proxy_marshal(iovec, 0, "dd", header.type, header.size); + assert(retval == 4 * 2); header.size += PROXY_HDR_SZ; retval = qemu_write_full(proxy->sockfd, iovec->iov_base, header.size);
This should always successfully write exactly two 32-bit integers. Make it clear with an assert(), like v9fs_receive_status() and v9fs_receive_response() already do when unmarshalling the same header. Fixes: Coverity CID 1438968 Signed-off-by: Greg Kurz <groug@kaod.org> --- hw/9pfs/9p-proxy.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)