vmd(8): send correct response on unpause error

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

vmd(8): send correct response on unpause error

Dave Voutila-2
If vmctl(8) sends an unpause request for a vm that doesn't exist, vmd(8)
should be responding with the IMSG_VMDOP_UNPAUSE_VM_RESPONSE imsg_type
with an ENOENT error code. (Similarly if the request comes from a user
without permissions to unpause, the error is EPERM but the imsg_type is
wrong.)

Since the handling for pause/unpause are the same code path, vmd(8) is
sending an IMSG_VMDOP_PAUSE_VM_RESPONSE in these situations (i.e. on an
error unpausing).

The below diff sets the cmd correctly based on the imsg being
processed.

For context, case statement in this switch block looks like:

        case IMSG_VMDOP_PAUSE_VM:
        case IMSG_VMDOP_UNPAUSE_VM:
                IMSG_SIZE_CHECK(imsg, &vid);
                memcpy(&vid, imsg->data, sizeof(vid));
                ...<the below diff>...

OK?

-dv


Index: usr.sbin/vmd/vmd.c
===================================================================
RCS file: /cvs/src/usr.sbin/vmd/vmd.c,v
retrieving revision 1.121
diff -u -p -r1.121 vmd.c
--- usr.sbin/vmd/vmd.c 29 Mar 2021 23:37:01 -0000 1.121
+++ usr.sbin/vmd/vmd.c 2 Apr 2021 23:06:47 -0000
@@ -203,20 +203,26 @@ vmd_dispatch_control(int fd, struct priv
  if (vid.vid_id == 0) {
  if ((vm = vm_getbyname(vid.vid_name)) == NULL) {
  res = ENOENT;
- cmd = IMSG_VMDOP_PAUSE_VM_RESPONSE;
+ cmd = imsg->hdr.type == IMSG_VMDOP_PAUSE_VM
+    ? IMSG_VMDOP_PAUSE_VM_RESPONSE
+    : IMSG_VMDOP_UNPAUSE_VM_RESPONSE;
  break;
  } else {
  vid.vid_id = vm->vm_vmid;
  }
  } else if ((vm = vm_getbyid(vid.vid_id)) == NULL) {
  res = ENOENT;
- cmd = IMSG_VMDOP_PAUSE_VM_RESPONSE;
+ cmd = imsg->hdr.type == IMSG_VMDOP_PAUSE_VM
+    ? IMSG_VMDOP_PAUSE_VM_RESPONSE
+    : IMSG_VMDOP_UNPAUSE_VM_RESPONSE;
  break;
  }
  if (vm_checkperm(vm, &vm->vm_params.vmc_owner,
     vid.vid_uid) != 0) {
  res = EPERM;
- cmd = IMSG_VMDOP_PAUSE_VM_RESPONSE;
+ cmd = imsg->hdr.type == IMSG_VMDOP_PAUSE_VM
+    ? IMSG_VMDOP_PAUSE_VM_RESPONSE
+    : IMSG_VMDOP_UNPAUSE_VM_RESPONSE;
  break;
  }
  proc_compose_imsg(ps, PROC_VMM, -1, imsg->hdr.type,

Reply | Threaded
Open this post in threaded view
|

Re: vmd(8): send correct response on unpause error

Mike Larkin-2
On Fri, Apr 02, 2021 at 07:14:34PM -0400, Dave Voutila wrote:

> If vmctl(8) sends an unpause request for a vm that doesn't exist, vmd(8)
> should be responding with the IMSG_VMDOP_UNPAUSE_VM_RESPONSE imsg_type
> with an ENOENT error code. (Similarly if the request comes from a user
> without permissions to unpause, the error is EPERM but the imsg_type is
> wrong.)
>
> Since the handling for pause/unpause are the same code path, vmd(8) is
> sending an IMSG_VMDOP_PAUSE_VM_RESPONSE in these situations (i.e. on an
> error unpausing).
>
> The below diff sets the cmd correctly based on the imsg being
> processed.
>
> For context, case statement in this switch block looks like:
>
> case IMSG_VMDOP_PAUSE_VM:
> case IMSG_VMDOP_UNPAUSE_VM:
> IMSG_SIZE_CHECK(imsg, &vid);
> memcpy(&vid, imsg->data, sizeof(vid));
>                 ...<the below diff>...
>
> OK?
>
> -dv
>

This is ok mlarkin@ if it wasn't already committed.

-ml

>
> Index: usr.sbin/vmd/vmd.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/vmd/vmd.c,v
> retrieving revision 1.121
> diff -u -p -r1.121 vmd.c
> --- usr.sbin/vmd/vmd.c 29 Mar 2021 23:37:01 -0000 1.121
> +++ usr.sbin/vmd/vmd.c 2 Apr 2021 23:06:47 -0000
> @@ -203,20 +203,26 @@ vmd_dispatch_control(int fd, struct priv
>   if (vid.vid_id == 0) {
>   if ((vm = vm_getbyname(vid.vid_name)) == NULL) {
>   res = ENOENT;
> - cmd = IMSG_VMDOP_PAUSE_VM_RESPONSE;
> + cmd = imsg->hdr.type == IMSG_VMDOP_PAUSE_VM
> +    ? IMSG_VMDOP_PAUSE_VM_RESPONSE
> +    : IMSG_VMDOP_UNPAUSE_VM_RESPONSE;
>   break;
>   } else {
>   vid.vid_id = vm->vm_vmid;
>   }
>   } else if ((vm = vm_getbyid(vid.vid_id)) == NULL) {
>   res = ENOENT;
> - cmd = IMSG_VMDOP_PAUSE_VM_RESPONSE;
> + cmd = imsg->hdr.type == IMSG_VMDOP_PAUSE_VM
> +    ? IMSG_VMDOP_PAUSE_VM_RESPONSE
> +    : IMSG_VMDOP_UNPAUSE_VM_RESPONSE;
>   break;
>   }
>   if (vm_checkperm(vm, &vm->vm_params.vmc_owner,
>      vid.vid_uid) != 0) {
>   res = EPERM;
> - cmd = IMSG_VMDOP_PAUSE_VM_RESPONSE;
> + cmd = imsg->hdr.type == IMSG_VMDOP_PAUSE_VM
> +    ? IMSG_VMDOP_PAUSE_VM_RESPONSE
> +    : IMSG_VMDOP_UNPAUSE_VM_RESPONSE;
>   break;
>   }
>   proc_compose_imsg(ps, PROC_VMM, -1, imsg->hdr.type,
>