| Message ID | 20211215222939.24738-2-raphael.norwitz@nutanix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Clean up error handling in libvhost-user memory mapping | expand |
On Wed, Dec 15, 2021 at 10:29:48PM +0000, Raphael Norwitz wrote: > Signed-off-by: Raphael Norwitz <raphael.norwitz@nutanix.com> > --- > subprojects/libvhost-user/libvhost-user.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c > index 787f4d2d4f..573212a83b 100644 > --- a/subprojects/libvhost-user/libvhost-user.c > +++ b/subprojects/libvhost-user/libvhost-user.c > @@ -801,6 +801,12 @@ vu_rem_mem_reg(VuDev *dev, VhostUserMsg *vmsg) { > VuDevRegion shadow_regions[VHOST_USER_MAX_RAM_SLOTS] = {}; > VhostUserMemoryRegion m = vmsg->payload.memreg.region, *msg_region = &m; > > + if (vmsg->fd_num != 1 || > + vmsg->size != sizeof(vmsg->payload.memreg)) { > + vu_panic(dev, "VHOST_USER_REM_MEM_REG received multiple regions"); > + return true; Most vu_panic() callers return false to indicate that a reply does not need to be sent. When the return value is true vu_dispatch() sends a response, which we don't want. Note that vu_dispatch() returns true (success) when the message handler function returns false. The success/failure behavior should probably be separated from the reply_requested behavior :(. Anyway, returning false is probably appropriate here. Stefan
On Wed, Jan 05, 2022 at 11:00:35AM +0000, Stefan Hajnoczi wrote: > On Wed, Dec 15, 2021 at 10:29:48PM +0000, Raphael Norwitz wrote: > > Signed-off-by: Raphael Norwitz <raphael.norwitz@nutanix.com> > > --- > > subprojects/libvhost-user/libvhost-user.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c > > index 787f4d2d4f..573212a83b 100644 > > --- a/subprojects/libvhost-user/libvhost-user.c > > +++ b/subprojects/libvhost-user/libvhost-user.c > > @@ -801,6 +801,12 @@ vu_rem_mem_reg(VuDev *dev, VhostUserMsg *vmsg) { > > VuDevRegion shadow_regions[VHOST_USER_MAX_RAM_SLOTS] = {}; > > VhostUserMemoryRegion m = vmsg->payload.memreg.region, *msg_region = &m; > > > > + if (vmsg->fd_num != 1 || > > + vmsg->size != sizeof(vmsg->payload.memreg)) { > > + vu_panic(dev, "VHOST_USER_REM_MEM_REG received multiple regions"); > > + return true; > > Most vu_panic() callers return false to indicate that a reply does not > need to be sent. When the return value is true vu_dispatch() sends a > response, which we don't want. > > Note that vu_dispatch() returns true (success) when the message handler > function returns false. The success/failure behavior should probably be > separated from the reply_requested behavior :(. > > Anyway, returning false is probably appropriate here. > Ack - I'll fix it in all the patches. > Stefan
diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c index 787f4d2d4f..573212a83b 100644 --- a/subprojects/libvhost-user/libvhost-user.c +++ b/subprojects/libvhost-user/libvhost-user.c @@ -801,6 +801,12 @@ vu_rem_mem_reg(VuDev *dev, VhostUserMsg *vmsg) { VuDevRegion shadow_regions[VHOST_USER_MAX_RAM_SLOTS] = {}; VhostUserMemoryRegion m = vmsg->payload.memreg.region, *msg_region = &m; + if (vmsg->fd_num != 1 || + vmsg->size != sizeof(vmsg->payload.memreg)) { + vu_panic(dev, "VHOST_USER_REM_MEM_REG received multiple regions"); + return true; + } + DPRINT("Removing region:\n"); DPRINT(" guest_phys_addr: 0x%016"PRIx64"\n", msg_region->guest_phys_addr);
Signed-off-by: Raphael Norwitz <raphael.norwitz@nutanix.com> --- subprojects/libvhost-user/libvhost-user.c | 6 ++++++ 1 file changed, 6 insertions(+)