Re: hostctl: Change from fixed length to variable length
ok yasuoka
On Fri, 06 Jan 2023 15:14:05 +0900 (JST)
Masato Asou wrote:
> I have updated my patch.
>
> From: YASUOKA Masahiko
> Date: Tue, 27 Dec 2022 11:58:34 +0900 (JST)
>
>> After diff, it doesn't use PAGE_SIZE any more. And VMware software
>> limit seems 1MB and changable by its configuration(*1). So we can't
>> say PVBUS_KVOP_MAXSIZE is enough.
>>
>> + * - Known pv backends other than vmware has a hard limit smaller than
>> + * PVBUS_KVOP_MAXSIZE in their messaging. vmware has a software
>> + * limit at 1MB, but current open-vm-tools has a limit at 64KB
>> + * (=PVBUS_KVOP_MAXSIZE).
>
> Use this comment.
>
> From: YASUOKA Masahiko
> Date: Tue, 27 Dec 2022 12:23:39 +0900 (JST)
>
>> Also I don't think replacing strlcat() by an own calculation is necessary.
>>
>> diff --git a/sys/dev/pv/xenstore.c b/sys/dev/pv/xenstore.c
>> index 494eb40bfb0..01ecebdf4af 100644
>> --- a/sys/dev/pv/xenstore.c
>> +++ b/sys/dev/pv/xenstore.c
>> @@ -1116,11 +1116,16 @@ xs_kvop(void *xsc, int op, char *key, char *value,
>> size_t valuelen)
>> /* FALLTHROUGH */
>> case XS_LIST:
>> for (i = 0; i < iov_cnt; i++) {
>> -if (i && strlcat(value, "\n", valuelen) >= valuelen)
>> +if (i > 0 && strlcat(value, "\n", valuelen) >=
>> +valuelen) {
>> +error = ERANGE;
>> break;
>> +}
>> if (strlcat(value, iovp[i].iov_base,
>> -valuelen) >= valuelen)
>> +valuelen) >= valuelen) {
>> +error = ERANGE;
>> break;
>> +}
>> }
>> xs_resfree(, iovp, iov_cnt);
>> break;
>> @@ -1128,5 +1133,5 @@ xs_kvop(void *xsc, int op, char *key, char *value,
>> size_t valuelen)
>> break;
>> }
>>
>> -return (0);
>> +return (error);
>> }
>>
>
> And use above diff.
>
> comment, ok?
> --
> ASOU Masato
>
> Index: share/man/man4/pvbus.4
> ===
> RCS file: /cvs/src/share/man/man4/pvbus.4,v
> retrieving revision 1.14
> diff -u -p -r1.14 pvbus.4
> --- share/man/man4/pvbus.414 Jun 2017 12:42:09 - 1.14
> +++ share/man/man4/pvbus.45 Jan 2023 23:20:39 -
> @@ -125,6 +125,13 @@ Read the value from
> .Fa pvr_key
> and return it in
> .Fa pvr_value .
> +If
> +.Fa pvr_valuelen
> +is not enough for the value,
> +the command will fail and
> +.Xr errno 2
> +is set to
> +.Er ERANGE .
> .It Dv PVBUSIOC_KVTYPE
> Return the type of the attached hypervisor interface as a string in
> .Fa pvr_key ;
> Index: sys/dev/pv/hypervic.c
> ===
> RCS file: /cvs/src/sys/dev/pv/hypervic.c,v
> retrieving revision 1.17
> diff -u -p -r1.17 hypervic.c
> --- sys/dev/pv/hypervic.c 8 Sep 2022 10:22:06 - 1.17
> +++ sys/dev/pv/hypervic.c 5 Jan 2023 23:20:40 -
> @@ -1151,11 +1151,12 @@ hv_kvop(void *arg, int op, char *key, ch
> kvpl = >kvp_pool[pool];
> if (strlen(key) == 0) {
> for (next = 0; next < MAXPOOLENTS; next++) {
> - if ((val + vallen < vp + HV_KVP_MAX_KEY_SIZE / 2) ||
> - kvp_pool_keys(kvpl, next, vp, ))
> + if (val + vallen < vp + HV_KVP_MAX_KEY_SIZE / 2)
> + return (ERANGE);
> + if (kvp_pool_keys(kvpl, next, vp, ))
> goto out;
> if (strlcat(val, "\n", vallen) >= vallen)
> - goto out;
> + return (ERANGE);
> vp += keylen;
> }
> out:
> Index: sys/dev/pv/pvbus.c
> ===
> RCS file: /cvs/src/sys/dev/pv/pvbus.c,v
> retrieving revision 1.26
> diff -u -p -r1.26 pvbus.c
> --- sys/dev/pv/pvbus.c8 Dec 2022 05:45:36 - 1.26
> +++ sys/dev/pv/pvbus.c5 Jan 2023 23:20:40 -
> @@ -399,13 +399,14 @@ pvbusgetstr(size_t srclen, const char *s
>
> /*
>* Reject size that is too short or obviously too long:
> - * - at least one byte for the nul terminator.
> - * - PAGE_SIZE is an arbitrary value, but known pv backends seem
> - * to have a hard (PAGE_SIZE - x) limit in their messaging.
> + * - Known pv backends other than vmware have a hard limit smaller than
> + * PVBUS_KVOP_MAXSIZE in their messaging. vmware has a software
> + * limit at 1MB, but current open-vm-tools has a limit at 64KB
> + * (=PVBUS_KVOP_MAXSIZE).
>*/
> if (srclen < 1)
> return (EINVAL);
> - else if (srclen > PAGE_SIZE)
> + else if (srclen > PVBUS_KVOP_MAXSIZE)
> return (ENAMETOOLONG);
>
Re: hostctl: Change from fixed length to variable length
I have updated my patch.
From: YASUOKA Masahiko
Date: Tue, 27 Dec 2022 11:58:34 +0900 (JST)
> After diff, it doesn't use PAGE_SIZE any more. And VMware software
> limit seems 1MB and changable by its configuration(*1). So we can't
> say PVBUS_KVOP_MAXSIZE is enough.
>
> + * - Known pv backends other than vmware has a hard limit smaller than
> + * PVBUS_KVOP_MAXSIZE in their messaging. vmware has a software
> + * limit at 1MB, but current open-vm-tools has a limit at 64KB
> + * (=PVBUS_KVOP_MAXSIZE).
Use this comment.
From: YASUOKA Masahiko
Date: Tue, 27 Dec 2022 12:23:39 +0900 (JST)
> Also I don't think replacing strlcat() by an own calculation is necessary.
>
> diff --git a/sys/dev/pv/xenstore.c b/sys/dev/pv/xenstore.c
> index 494eb40bfb0..01ecebdf4af 100644
> --- a/sys/dev/pv/xenstore.c
> +++ b/sys/dev/pv/xenstore.c
> @@ -1116,11 +1116,16 @@ xs_kvop(void *xsc, int op, char *key, char *value,
> size_t valuelen)
> /* FALLTHROUGH */
> case XS_LIST:
> for (i = 0; i < iov_cnt; i++) {
> - if (i && strlcat(value, "\n", valuelen) >= valuelen)
> + if (i > 0 && strlcat(value, "\n", valuelen) >=
> + valuelen) {
> + error = ERANGE;
> break;
> + }
> if (strlcat(value, iovp[i].iov_base,
> - valuelen) >= valuelen)
> + valuelen) >= valuelen) {
> + error = ERANGE;
> break;
> + }
> }
> xs_resfree(, iovp, iov_cnt);
> break;
> @@ -1128,5 +1133,5 @@ xs_kvop(void *xsc, int op, char *key, char *value,
> size_t valuelen)
> break;
> }
>
> - return (0);
> + return (error);
> }
>
And use above diff.
comment, ok?
--
ASOU Masato
Index: share/man/man4/pvbus.4
===
RCS file: /cvs/src/share/man/man4/pvbus.4,v
retrieving revision 1.14
diff -u -p -r1.14 pvbus.4
--- share/man/man4/pvbus.4 14 Jun 2017 12:42:09 - 1.14
+++ share/man/man4/pvbus.4 5 Jan 2023 23:20:39 -
@@ -125,6 +125,13 @@ Read the value from
.Fa pvr_key
and return it in
.Fa pvr_value .
+If
+.Fa pvr_valuelen
+is not enough for the value,
+the command will fail and
+.Xr errno 2
+is set to
+.Er ERANGE .
.It Dv PVBUSIOC_KVTYPE
Return the type of the attached hypervisor interface as a string in
.Fa pvr_key ;
Index: sys/dev/pv/hypervic.c
===
RCS file: /cvs/src/sys/dev/pv/hypervic.c,v
retrieving revision 1.17
diff -u -p -r1.17 hypervic.c
--- sys/dev/pv/hypervic.c 8 Sep 2022 10:22:06 - 1.17
+++ sys/dev/pv/hypervic.c 5 Jan 2023 23:20:40 -
@@ -1151,11 +1151,12 @@ hv_kvop(void *arg, int op, char *key, ch
kvpl = >kvp_pool[pool];
if (strlen(key) == 0) {
for (next = 0; next < MAXPOOLENTS; next++) {
- if ((val + vallen < vp + HV_KVP_MAX_KEY_SIZE / 2) ||
- kvp_pool_keys(kvpl, next, vp, ))
+ if (val + vallen < vp + HV_KVP_MAX_KEY_SIZE / 2)
+ return (ERANGE);
+ if (kvp_pool_keys(kvpl, next, vp, ))
goto out;
if (strlcat(val, "\n", vallen) >= vallen)
- goto out;
+ return (ERANGE);
vp += keylen;
}
out:
Index: sys/dev/pv/pvbus.c
===
RCS file: /cvs/src/sys/dev/pv/pvbus.c,v
retrieving revision 1.26
diff -u -p -r1.26 pvbus.c
--- sys/dev/pv/pvbus.c 8 Dec 2022 05:45:36 - 1.26
+++ sys/dev/pv/pvbus.c 5 Jan 2023 23:20:40 -
@@ -399,13 +399,14 @@ pvbusgetstr(size_t srclen, const char *s
/*
* Reject size that is too short or obviously too long:
-* - at least one byte for the nul terminator.
-* - PAGE_SIZE is an arbitrary value, but known pv backends seem
-* to have a hard (PAGE_SIZE - x) limit in their messaging.
+* - Known pv backends other than vmware have a hard limit smaller than
+* PVBUS_KVOP_MAXSIZE in their messaging. vmware has a software
+* limit at 1MB, but current open-vm-tools has a limit at 64KB
+* (=PVBUS_KVOP_MAXSIZE).
*/
if (srclen < 1)
return (EINVAL);
- else if (srclen > PAGE_SIZE)
+ else if (srclen > PVBUS_KVOP_MAXSIZE)
return (ENAMETOOLONG);
*dstp = dst = malloc(srclen + 1, M_TEMP, M_WAITOK | M_ZERO);
Index: sys/dev/pv/pvvar.h
===
RCS file: /cvs/src/sys/dev/pv/pvvar.h,v
Re: hostctl: Change from fixed length to variable length
Hi,
Sorry for separating emails.
On Tue, 27 Dec 2022 11:58:34 +0900 (JST)
YASUOKA Masahiko wrote:
>> @@ -1115,12 +1116,19 @@ xs_kvop(void *xsc, int op, char *key, char *value,
>> size_t valuelen)
>> }
>> /* FALLTHROUGH */
>> case XS_LIST:
>> -for (i = 0; i < iov_cnt; i++) {
>> -if (i && strlcat(value, "\n", valuelen) >= valuelen)
>> -break;
>> -if (strlcat(value, iovp[i].iov_base,
>> -valuelen) >= valuelen)
>> +for (i = pos = 0; i < iov_cnt; i++) {
>> +if (i) {
>
> this is come from the previous, but I prefer comparing with 0
>
> + if (i > 0) {
>
>> +if (pos + 1 >= valuelen) {
>> +error = ERANGE;
>> +break;
>> +}
>> +value[pos++] = '\n';
>> +}
>> +if (strlen(iovp[i].iov_base) >= valuelen) {
>> +error = ERANGE;
>> break;
>> +}
>> +pos += strlcat([pos], iovp[i].iov_base, valuelen
>> - pos);
>> }
>> xs_resfree(, iovp, iov_cnt);
>> break;
>
Also I don't think replacing strlcat() by an own calculation is necessary.
diff --git a/sys/dev/pv/xenstore.c b/sys/dev/pv/xenstore.c
index 494eb40bfb0..01ecebdf4af 100644
--- a/sys/dev/pv/xenstore.c
+++ b/sys/dev/pv/xenstore.c
@@ -1116,11 +1116,16 @@ xs_kvop(void *xsc, int op, char *key, char *value,
size_t valuelen)
/* FALLTHROUGH */
case XS_LIST:
for (i = 0; i < iov_cnt; i++) {
- if (i && strlcat(value, "\n", valuelen) >= valuelen)
+ if (i > 0 && strlcat(value, "\n", valuelen) >=
+ valuelen) {
+ error = ERANGE;
break;
+ }
if (strlcat(value, iovp[i].iov_base,
- valuelen) >= valuelen)
+ valuelen) >= valuelen) {
+ error = ERANGE;
break;
+ }
}
xs_resfree(, iovp, iov_cnt);
break;
@@ -1128,5 +1133,5 @@ xs_kvop(void *xsc, int op, char *key, char *value, size_t
valuelen)
break;
}
- return (0);
+ return (error);
}
Re: hostctl: Change from fixed length to variable length
Hi,
On Mon, 26 Dec 2022 13:37:45 +0900 (JST)
Masato Asou wrote:
> My new patch is not returned value length from ioctl() system call
> when read the KEY's value.
>
> The hostctl command allocate 4k bytes memory for store the value. Then
> read the value by ioctl() system call. If ioctl() returned -1 end
> errno is ERANGE, then hostctl comannd reallocate twice as much space.
I will support this direction.
> The upper limit is PVBUS_KVOP_MAXSIZE (64k bytes).
Let me note that open-vm-tools also has the same hard-coded limit
https://github.com/vmware/open-vm-tools/blob/162eba6ab52d664551ffae343ef7e9a7f211ca69/open-vm-tools/lib/include/guest_msg_def.h#L108
There are 2 small feedbacks
> diff --git a/sys/dev/pv/pvbus.c b/sys/dev/pv/pvbus.c
> index 5f7c4b57fe0..c76a9e81444 100644
> --- a/sys/dev/pv/pvbus.c
> +++ b/sys/dev/pv/pvbus.c
> @@ -400,12 +400,12 @@ pvbusgetstr(size_t srclen, const char *src, char **dstp)
> /*
>* Reject size that is too short or obviously too long:
>* - at least one byte for the nul terminator.
> - * - PAGE_SIZE is an arbitrary value, but known pv backends seem
> - * to have a hard (PAGE_SIZE - x) limit in their messaging.
> + * - PVBUS_KVOP_MAXSIZE is an arbitrary value, but known pv backends
> + * seem to have a hard (PAGE_SIZE - x) limit in their messaging.
After diff, it doesn't use PAGE_SIZE any more. And VMware software
limit seems 1MB and changable by its configuration(*1). So we can't
say PVBUS_KVOP_MAXSIZE is enough.
+* - Known pv backends other than vmware has a hard limit smaller than
+* PVBUS_KVOP_MAXSIZE in their messaging. vmware has a software
+* limit at 1MB, but current open-vm-tools has a limit at 64KB
+* (=PVBUS_KVOP_MAXSIZE).
*1
https://docs.vmware.com/en/VMware-vSphere/7.0/com.vmware.vsphere.security.doc/GUID-91BF834E-CB92-4014-8CF7-29CE40F3E8A3.html
>*/
> if (srclen < 1)
> return (EINVAL);
> - else if (srclen > PAGE_SIZE)
> + else if (srclen > PVBUS_KVOP_MAXSIZE)
> return (ENAMETOOLONG);
>
> *dstp = dst = malloc(srclen + 1, M_TEMP, M_WAITOK | M_ZERO);
> @@ -1115,12 +1116,19 @@ xs_kvop(void *xsc, int op, char *key, char *value,
> size_t valuelen)
> }
> /* FALLTHROUGH */
> case XS_LIST:
> - for (i = 0; i < iov_cnt; i++) {
> - if (i && strlcat(value, "\n", valuelen) >= valuelen)
> - break;
> - if (strlcat(value, iovp[i].iov_base,
> - valuelen) >= valuelen)
> + for (i = pos = 0; i < iov_cnt; i++) {
> + if (i) {
this is come from the previous, but I prefer comparing with 0
+ if (i > 0) {
> + if (pos + 1 >= valuelen) {
> + error = ERANGE;
> + break;
> + }
> + value[pos++] = '\n';
> + }
> + if (strlen(iovp[i].iov_base) >= valuelen) {
> + error = ERANGE;
> break;
> + }
> + pos += strlcat([pos], iovp[i].iov_base, valuelen
> - pos);
> }
> xs_resfree(, iovp, iov_cnt);
> break;
Re: hostctl: Change from fixed length to variable length
I was rewrited the patch for hostctl command and sys/dev/pvbus.
From: "Theo de Raadt"
Date: Tue, 11 Oct 2022 19:09:42 -0600
> An example of this mechanism is SIOCGIFCONF. The ioctl passes a pointer
> to a struct containing length & pointer to data. See net/if.c ifconf()
> There are other similar schemes, but they all come down to asking the
> kernel for the size and then doing a 2nd ioctl.
>
> Or a 3rd or more calls, in case the value has changed in the meantime and
> grown even further, but userland can realloc() the storage until it wins.
>
My new patch is not returned value length from ioctl() system call
when read the KEY's value.
The hostctl command allocate 4k bytes memory for store the value. Then
read the value by ioctl() system call. If ioctl() returned -1 end
errno is ERANGE, then hostctl comannd reallocate twice as much space.
The upper limit is PVBUS_KVOP_MAXSIZE (64k bytes).
Check the patch, please.
comment, ok?
--
ASOU Masato
diff --git a/share/man/man4/pvbus.4 b/share/man/man4/pvbus.4
index 8d67809e9c0..3c6681decf0 100644
--- a/share/man/man4/pvbus.4
+++ b/share/man/man4/pvbus.4
@@ -125,6 +125,13 @@ Read the value from
.Fa pvr_key
and return it in
.Fa pvr_value .
+If
+.Fa pvr_valuelen
+is not enough for the value,
+the command will fail and
+.Xr errno 2
+is set to
+.Er ERANGE .
.It Dv PVBUSIOC_KVTYPE
Return the type of the attached hypervisor interface as a string in
.Fa pvr_key ;
diff --git a/sys/dev/pv/hypervic.c b/sys/dev/pv/hypervic.c
index 9c7f70dd96f..a7455d03e5b 100644
--- a/sys/dev/pv/hypervic.c
+++ b/sys/dev/pv/hypervic.c
@@ -1151,11 +1151,12 @@ hv_kvop(void *arg, int op, char *key, char *val, size_t
vallen)
kvpl = >kvp_pool[pool];
if (strlen(key) == 0) {
for (next = 0; next < MAXPOOLENTS; next++) {
- if ((val + vallen < vp + HV_KVP_MAX_KEY_SIZE / 2) ||
- kvp_pool_keys(kvpl, next, vp, ))
+ if (val + vallen < vp + HV_KVP_MAX_KEY_SIZE / 2)
+ return (ERANGE);
+ if (kvp_pool_keys(kvpl, next, vp, ))
goto out;
if (strlcat(val, "\n", vallen) >= vallen)
- goto out;
+ return (ERANGE);
vp += keylen;
}
out:
diff --git a/sys/dev/pv/pvbus.c b/sys/dev/pv/pvbus.c
index 5f7c4b57fe0..c76a9e81444 100644
--- a/sys/dev/pv/pvbus.c
+++ b/sys/dev/pv/pvbus.c
@@ -400,12 +400,12 @@ pvbusgetstr(size_t srclen, const char *src, char **dstp)
/*
* Reject size that is too short or obviously too long:
* - at least one byte for the nul terminator.
-* - PAGE_SIZE is an arbitrary value, but known pv backends seem
-* to have a hard (PAGE_SIZE - x) limit in their messaging.
+* - PVBUS_KVOP_MAXSIZE is an arbitrary value, but known pv backends
+* seem to have a hard (PAGE_SIZE - x) limit in their messaging.
*/
if (srclen < 1)
return (EINVAL);
- else if (srclen > PAGE_SIZE)
+ else if (srclen > PVBUS_KVOP_MAXSIZE)
return (ENAMETOOLONG);
*dstp = dst = malloc(srclen + 1, M_TEMP, M_WAITOK | M_ZERO);
diff --git a/sys/dev/pv/pvvar.h b/sys/dev/pv/pvvar.h
index 4e23ae52bd5..333d4fddf9c 100644
--- a/sys/dev/pv/pvvar.h
+++ b/sys/dev/pv/pvvar.h
@@ -30,6 +30,8 @@ struct pvbus_req {
#define PVBUSIOC_KVWRITE _IOWR('V', 2, struct pvbus_req)
#define PVBUSIOC_TYPE _IOWR('V', 3, struct pvbus_req)
+#definePVBUS_KVOP_MAXSIZE (64 * 1024)
+
#ifdef _KERNEL
enum {
PVBUS_KVM,
diff --git a/sys/dev/pv/xenstore.c b/sys/dev/pv/xenstore.c
index 494eb40bfb0..c32f1cea7d7 100644
--- a/sys/dev/pv/xenstore.c
+++ b/sys/dev/pv/xenstore.c
@@ -1072,6 +1072,7 @@ xs_kvop(void *xsc, int op, char *key, char *value, size_t
valuelen)
struct xs_transaction xst;
struct iovec iov, *iovp =
int error = 0, iov_cnt = 0, cmd, i;
+ size_t pos;
switch (op) {
case PVBUS_KVWRITE:
@@ -1115,12 +1116,19 @@ xs_kvop(void *xsc, int op, char *key, char *value,
size_t valuelen)
}
/* FALLTHROUGH */
case XS_LIST:
- for (i = 0; i < iov_cnt; i++) {
- if (i && strlcat(value, "\n", valuelen) >= valuelen)
- break;
- if (strlcat(value, iovp[i].iov_base,
- valuelen) >= valuelen)
+ for (i = pos = 0; i < iov_cnt; i++) {
+ if (i) {
+ if (pos + 1 >= valuelen) {
+ error = ERANGE;
+ break;
+ }
+ value[pos++] = '\n';
+ }
+ if (strlen(iovp[i].iov_base) >=
Re: hostctl: Change from fixed length to variable length
delete mbuhl from Cc:.
From: YASUOKA Masahiko
Date: Sat, 19 Nov 2022 16:37:47 +0900 (JST)
> On Sat, 19 Nov 2022 14:41:18 +0900 (JST)
> YASUOKA Masahiko wrote:
>> On Wed, 12 Oct 2022 07:58:20 +0900 (JST)
>> YASUOKA Masahiko wrote:
>>> On Wed, 05 Oct 2022 13:37:35 +0900 (JST)
>>> Masato Asou wrote:
From: "Theo de Raadt"
Date: Tue, 04 Oct 2022 21:58:13 -0600
> Userland may not ask the kernel to allocate such a huge object. The
> kernel address space is quite small. First off, huge allocations will
> fail.
>
> But it is worse -- the small kernel address space is shared for many
> purposes, so large allocations will harm the other subsystems.
>
> As written, this diff is too dangerous. Arbitrary allocation inside
> the kernel is not reasonable. object sizes requested by userland must
> be small, or the operations must be cut up, which does create impact
> on atomicity or other things.
As you pointed out, it is not a good idea to allocate large spaces
in kernel.
Would it be better to keep the current fixed length?
>>>
>>> Currently the value on VMware may be truncated silently. It's simply
>>> broken. I think we should fix it by having a way to know if the value
>>> is reached the limit.
>>>
>>> Also I think we should be able to pass larger size of data. Since at
>>> least on VMware, people is useing for parameters when deployment
>>> through OVF tamplate. Sometimes the parameter includes large data
>>> like X.509 certificate.
>>>
>>> https://docs.vmware.com/en/VMware-vSphere/7.0/com.vmware.vsphere.vm_admin.doc/GUID-D0F9E3B9-B77B-4DEF-A982-49B9F6358FF3.html
>>>
>>> What do you think?
>>>
Prepare a variable like kern.maxpvbus and default it to
4096. Futhermore, how about free() after copyout() to user space?
>>>
>>> I suppose we can use the space prepared by the userland directly.
>>
>> I admit there is no way to use the user space directly in case of the
>> vmware.
>>
>> But current vmt(4) uses the buffer in vmt_softc for all RPC commands.
>> The buffer seems to have beeen created for RPC done by the tclo
>> process. The tclo process executes RPC periodically, so having a long
>> term buffer does make sense.
>>
>> On the otherhand, pvbus(4) prepares a buffer for
>> PVBUSIOC_KV{READ|WRITE} and pass it to the driver handler. So vmt(4)
>> can use the buffer.
>>
>> The diff is to change vmt(4) to use the buffer given by pvbuf(4) for
>> the rpc output directly. Also make it return EMSGSIZE when the buffer
>> is not enough instead of truncating silently.
>>
>> The diff is first step. We need to more hack for pvbus(4) and
>> vmt(4). For example, the buffer size pvbuf(4) allocates is not enough
>> to store the size user requested, since vmt(4) neeeds extra 2 bytes
>> for the RPC output.
>>
>> +bcopy(value + 2, value, valuelen - 2);
>>
>>
>> But I'd like to commit this first.
>>
>> ok?
>
> sorry, using existing vm_rpc_send_rpci_tx_buf() or
> vm_rpci_response_successful() wasn't good idea and had a bug.
>
> Let me update the diff.
ok asou@
> Index: sys/dev/pv/vmt.c
> ===
> RCS file: /var/cvs/openbsd/src/sys/dev/pv/vmt.c,v
> retrieving revision 1.26
> diff -u -p -r1.26 vmt.c
> --- sys/dev/pv/vmt.c 8 Sep 2022 10:22:06 - 1.26
> +++ sys/dev/pv/vmt.c 19 Nov 2022 07:32:47 -
> @@ -491,9 +491,12 @@ int
> vmt_kvop(void *arg, int op, char *key, char *value, size_t valuelen)
> {
> struct vmt_softc *sc = arg;
> - char *buf = NULL, *ptr;
> + struct vm_rpc rpci;
> + char *buf = NULL;
> size_t bufsz;
> int error = 0;
> + uint32_t rlen;
> + uint16_t ack;
>
> bufsz = VMT_RPC_BUFLEN;
> buf = malloc(bufsz, M_TEMP, M_WAITOK | M_ZERO);
> @@ -520,25 +523,52 @@ vmt_kvop(void *arg, int op, char *key, c
> goto done;
> }
>
> - if (vm_rpc_send_rpci_tx(sc, "%s", buf) != 0) {
> - DPRINTF("%s: error sending command: %s\n", DEVNAME(sc), buf);
> + if (vm_rpc_open(, VM_RPC_OPEN_RPCI) != 0) {
> + DPRINTF("%s: rpci channel open failed\n", DEVNAME(sc));
> sc->sc_rpc_error = 1;
> error = EIO;
> goto done;
> }
>
> - if (vm_rpci_response_successful(sc) == 0) {
> - DPRINTF("%s: host rejected command: %s\n", DEVNAME(sc), buf);
> - error = EINVAL;
> + if (vm_rpc_send(, buf, bufsz) != 0) {
> + DPRINTF("%s: unable to send rpci command\n", DEVNAME(sc));
> + sc->sc_rpc_error = 1;
> + error = EIO;
> goto done;
> }
>
> - /* skip response that was tested in vm_rpci_response_successful() */
> - ptr = sc->sc_rpc_buf + 2;
> + if (vm_rpc_get_length(, , ) != 0) {
> + DPRINTF("%s: failed to get length of rpci response data\n",
> + DEVNAME(sc));
> + sc->sc_rpc_error =
Re: hostctl: Change from fixed length to variable length
On Sat, 19 Nov 2022 14:41:18 +0900 (JST)
YASUOKA Masahiko wrote:
> On Wed, 12 Oct 2022 07:58:20 +0900 (JST)
> YASUOKA Masahiko wrote:
>> On Wed, 05 Oct 2022 13:37:35 +0900 (JST)
>> Masato Asou wrote:
>>> From: "Theo de Raadt"
>>> Date: Tue, 04 Oct 2022 21:58:13 -0600
Userland may not ask the kernel to allocate such a huge object. The
kernel address space is quite small. First off, huge allocations will
fail.
But it is worse -- the small kernel address space is shared for many
purposes, so large allocations will harm the other subsystems.
As written, this diff is too dangerous. Arbitrary allocation inside
the kernel is not reasonable. object sizes requested by userland must
be small, or the operations must be cut up, which does create impact
on atomicity or other things.
>>>
>>> As you pointed out, it is not a good idea to allocate large spaces
>>> in kernel.
>>>
>>> Would it be better to keep the current fixed length?
>>
>> Currently the value on VMware may be truncated silently. It's simply
>> broken. I think we should fix it by having a way to know if the value
>> is reached the limit.
>>
>> Also I think we should be able to pass larger size of data. Since at
>> least on VMware, people is useing for parameters when deployment
>> through OVF tamplate. Sometimes the parameter includes large data
>> like X.509 certificate.
>>
>> https://docs.vmware.com/en/VMware-vSphere/7.0/com.vmware.vsphere.vm_admin.doc/GUID-D0F9E3B9-B77B-4DEF-A982-49B9F6358FF3.html
>>
>> What do you think?
>>
>>> Prepare a variable like kern.maxpvbus and default it to
>>> 4096. Futhermore, how about free() after copyout() to user space?
>>
>> I suppose we can use the space prepared by the userland directly.
>
> I admit there is no way to use the user space directly in case of the
> vmware.
>
> But current vmt(4) uses the buffer in vmt_softc for all RPC commands.
> The buffer seems to have beeen created for RPC done by the tclo
> process. The tclo process executes RPC periodically, so having a long
> term buffer does make sense.
>
> On the otherhand, pvbus(4) prepares a buffer for
> PVBUSIOC_KV{READ|WRITE} and pass it to the driver handler. So vmt(4)
> can use the buffer.
>
> The diff is to change vmt(4) to use the buffer given by pvbuf(4) for
> the rpc output directly. Also make it return EMSGSIZE when the buffer
> is not enough instead of truncating silently.
>
> The diff is first step. We need to more hack for pvbus(4) and
> vmt(4). For example, the buffer size pvbuf(4) allocates is not enough
> to store the size user requested, since vmt(4) neeeds extra 2 bytes
> for the RPC output.
>
> + bcopy(value + 2, value, valuelen - 2);
>
>
> But I'd like to commit this first.
>
> ok?
sorry, using existing vm_rpc_send_rpci_tx_buf() or
vm_rpci_response_successful() wasn't good idea and had a bug.
Let me update the diff.
Index: sys/dev/pv/vmt.c
===
RCS file: /var/cvs/openbsd/src/sys/dev/pv/vmt.c,v
retrieving revision 1.26
diff -u -p -r1.26 vmt.c
--- sys/dev/pv/vmt.c8 Sep 2022 10:22:06 - 1.26
+++ sys/dev/pv/vmt.c19 Nov 2022 07:32:47 -
@@ -491,9 +491,12 @@ int
vmt_kvop(void *arg, int op, char *key, char *value, size_t valuelen)
{
struct vmt_softc *sc = arg;
- char *buf = NULL, *ptr;
+ struct vm_rpc rpci;
+ char *buf = NULL;
size_t bufsz;
int error = 0;
+ uint32_t rlen;
+ uint16_t ack;
bufsz = VMT_RPC_BUFLEN;
buf = malloc(bufsz, M_TEMP, M_WAITOK | M_ZERO);
@@ -520,25 +523,52 @@ vmt_kvop(void *arg, int op, char *key, c
goto done;
}
- if (vm_rpc_send_rpci_tx(sc, "%s", buf) != 0) {
- DPRINTF("%s: error sending command: %s\n", DEVNAME(sc), buf);
+ if (vm_rpc_open(, VM_RPC_OPEN_RPCI) != 0) {
+ DPRINTF("%s: rpci channel open failed\n", DEVNAME(sc));
sc->sc_rpc_error = 1;
error = EIO;
goto done;
}
- if (vm_rpci_response_successful(sc) == 0) {
- DPRINTF("%s: host rejected command: %s\n", DEVNAME(sc), buf);
- error = EINVAL;
+ if (vm_rpc_send(, buf, bufsz) != 0) {
+ DPRINTF("%s: unable to send rpci command\n", DEVNAME(sc));
+ sc->sc_rpc_error = 1;
+ error = EIO;
goto done;
}
- /* skip response that was tested in vm_rpci_response_successful() */
- ptr = sc->sc_rpc_buf + 2;
+ if (vm_rpc_get_length(, , ) != 0) {
+ DPRINTF("%s: failed to get length of rpci response data\n",
+ DEVNAME(sc));
+ sc->sc_rpc_error = 1;
+ error = EIO;
+ goto done;
+ }
- /* might truncate, copy anyway but return error */
- if (strlcpy(value, ptr, valuelen) >= valuelen)
- error =
Re: hostctl: Change from fixed length to variable length
On Wed, 12 Oct 2022 07:58:20 +0900 (JST)
YASUOKA Masahiko wrote:
> On Wed, 05 Oct 2022 13:37:35 +0900 (JST)
> Masato Asou wrote:
>> From: "Theo de Raadt"
>> Date: Tue, 04 Oct 2022 21:58:13 -0600
>>> Userland may not ask the kernel to allocate such a huge object. The
>>> kernel address space is quite small. First off, huge allocations will
>>> fail.
>>>
>>> But it is worse -- the small kernel address space is shared for many
>>> purposes, so large allocations will harm the other subsystems.
>>>
>>> As written, this diff is too dangerous. Arbitrary allocation inside
>>> the kernel is not reasonable. object sizes requested by userland must
>>> be small, or the operations must be cut up, which does create impact
>>> on atomicity or other things.
>>
>> As you pointed out, it is not a good idea to allocate large spaces
>> in kernel.
>>
>> Would it be better to keep the current fixed length?
>
> Currently the value on VMware may be truncated silently. It's simply
> broken. I think we should fix it by having a way to know if the value
> is reached the limit.
>
> Also I think we should be able to pass larger size of data. Since at
> least on VMware, people is useing for parameters when deployment
> through OVF tamplate. Sometimes the parameter includes large data
> like X.509 certificate.
>
> https://docs.vmware.com/en/VMware-vSphere/7.0/com.vmware.vsphere.vm_admin.doc/GUID-D0F9E3B9-B77B-4DEF-A982-49B9F6358FF3.html
>
> What do you think?
>
>> Prepare a variable like kern.maxpvbus and default it to
>> 4096. Futhermore, how about free() after copyout() to user space?
>
> I suppose we can use the space prepared by the userland directly.
I admit there is no way to use the user space directly in case of the
vmware.
But current vmt(4) uses the buffer in vmt_softc for all RPC commands.
The buffer seems to have beeen created for RPC done by the tclo
process. The tclo process executes RPC periodically, so having a long
term buffer does make sense.
On the otherhand, pvbus(4) prepares a buffer for
PVBUSIOC_KV{READ|WRITE} and pass it to the driver handler. So vmt(4)
can use the buffer.
The diff is to change vmt(4) to use the buffer given by pvbuf(4) for
the rpc output directly. Also make it return EMSGSIZE when the buffer
is not enough instead of truncating silently.
The diff is first step. We need to more hack for pvbus(4) and
vmt(4). For example, the buffer size pvbuf(4) allocates is not enough
to store the size user requested, since vmt(4) neeeds extra 2 bytes
for the RPC output.
+ bcopy(value + 2, value, valuelen - 2);
But I'd like to commit this first.
ok?
Index: sys/dev/pv/vmt.c
===
RCS file: /var/cvs/openbsd/src/sys/dev/pv/vmt.c,v
retrieving revision 1.26
diff -u -p -r1.26 vmt.c
--- sys/dev/pv/vmt.c8 Sep 2022 10:22:06 - 1.26
+++ sys/dev/pv/vmt.c19 Nov 2022 04:13:47 -
@@ -277,7 +277,8 @@ int vm_rpc_send(const struct vm_rpc *,
int vm_rpc_send_str(const struct vm_rpc *, const uint8_t *);
int vm_rpc_get_length(const struct vm_rpc *, uint32_t *, uint16_t *);
int vm_rpc_get_data(const struct vm_rpc *, char *, uint32_t, uint16_t);
-int vm_rpc_send_rpci_tx_buf(struct vmt_softc *, const uint8_t *, uint32_t);
+int vm_rpc_send_rpci_tx_buf(struct vmt_softc *, const uint8_t *, uint32_t,
+ uint8_t *, uint32_t);
int vm_rpc_send_rpci_tx(struct vmt_softc *, const char *, ...)
__attribute__((__format__(__kprintf__,2,3)));
int vm_rpci_response_successful(struct vmt_softc *);
@@ -491,7 +492,7 @@ int
vmt_kvop(void *arg, int op, char *key, char *value, size_t valuelen)
{
struct vmt_softc *sc = arg;
- char *buf = NULL, *ptr;
+ char *buf = NULL;
size_t bufsz;
int error = 0;
@@ -520,10 +521,9 @@ vmt_kvop(void *arg, int op, char *key, c
goto done;
}
- if (vm_rpc_send_rpci_tx(sc, "%s", buf) != 0) {
- DPRINTF("%s: error sending command: %s\n", DEVNAME(sc), buf);
+ if ((error = vm_rpc_send_rpci_tx_buf(sc, buf, strlen(buf), value,
+ valuelen)) != 0) {
sc->sc_rpc_error = 1;
- error = EIO;
goto done;
}
@@ -534,11 +534,7 @@ vmt_kvop(void *arg, int op, char *key, c
}
/* skip response that was tested in vm_rpci_response_successful() */
- ptr = sc->sc_rpc_buf + 2;
-
- /* might truncate, copy anyway but return error */
- if (strlcpy(value, ptr, valuelen) >= valuelen)
- error = ENOMEM;
+ bcopy(value + 2, value, valuelen - 2);
done:
free(buf, M_TEMP, bufsz);
@@ -1348,8 +1344,8 @@ vmt_nicinfo_task(void *data)
if (nic_info_size != sc->sc_nic_info_size ||
(memcmp(nic_info, sc->sc_nic_info, nic_info_size) != 0)) {
- if (vm_rpc_send_rpci_tx_buf(sc, nic_info,
- nic_info_size) != 0) {
+ if
Re: hostctl: Change from fixed length to variable length
YASUOKA Masahiko wrote: > Currently the value on VMware may be truncated silently. It's simply > broken. I think we should fix it by having a way to know if the value > is reached the limit. > > Also I think we should be able to pass larger size of data. Since at > least on VMware, people is useing for parameters when deployment > through OVF tamplate. Sometimes the parameter includes large data > like X.509 certificate. > > https://docs.vmware.com/en/VMware-vSphere/7.0/com.vmware.vsphere.vm_admin.doc/GUID-D0F9E3B9-B77B-4DEF-A982-49B9F6358FF3.html > > What do you think? > > > Prepare a variable like kern.maxpvbus and default it to > > 4096. Futhermore, how about free() after copyout() to user space? > > I suppose we can use the space prepared by the userland directly. An example of this mechanism is SIOCGIFCONF. The ioctl passes a pointer to a struct containing length & pointer to data. See net/if.c ifconf() There are other similar schemes, but they all come down to asking the kernel for the size and then doing a 2nd ioctl. Or a 3rd or more calls, in case the value has changed in the meantime and grown even further, but userland can realloc() the storage until it wins.
Re: hostctl: Change from fixed length to variable length
Hello, On Wed, 05 Oct 2022 13:37:35 +0900 (JST) Masato Asou wrote: > From: "Theo de Raadt" > Date: Tue, 04 Oct 2022 21:58:13 -0600 >> Userland may not ask the kernel to allocate such a huge object. The >> kernel address space is quite small. First off, huge allocations will >> fail. >> >> But it is worse -- the small kernel address space is shared for many >> purposes, so large allocations will harm the other subsystems. >> >> As written, this diff is too dangerous. Arbitrary allocation inside >> the kernel is not reasonable. object sizes requested by userland must >> be small, or the operations must be cut up, which does create impact >> on atomicity or other things. > > As you pointed out, it is not a good idea to allocate large spaces > in kernel. > > Would it be better to keep the current fixed length? Currently the value on VMware may be truncated silently. It's simply broken. I think we should fix it by having a way to know if the value is reached the limit. Also I think we should be able to pass larger size of data. Since at least on VMware, people is useing for parameters when deployment through OVF tamplate. Sometimes the parameter includes large data like X.509 certificate. https://docs.vmware.com/en/VMware-vSphere/7.0/com.vmware.vsphere.vm_admin.doc/GUID-D0F9E3B9-B77B-4DEF-A982-49B9F6358FF3.html What do you think? > Prepare a variable like kern.maxpvbus and default it to > 4096. Futhermore, how about free() after copyout() to user space? I suppose we can use the space prepared by the userland directly.
Re: hostctl: Change from fixed length to variable length
Masato Asou wrote: > As you pointed out, it is not a good idea to allocate large spaces > in kernel. > > Would it be better to keep the current fixed length? > > Prepare a variable like kern.maxpvbus and default it to > 4096. Futhermore, how about free() after copyout() to user space? Sorry I did not look further since I am not familiar with the subsystem or what the purpose of the change is. Hopefully someone else who uses it will speak up and you can come up with a compromise.
Re: hostctl: Change from fixed length to variable length
From: "Theo de Raadt"
Date: Tue, 04 Oct 2022 21:58:13 -0600
> Looking at these pieces:
>
> + sc->sc_rpc_buf = malloc(sc->sc_rpc_buflen, M_DEVBUF, M_NOWAIT);
> ...
> +vm_rpc_buf_realloc(struct vmt_softc *sc, size_t len)
> +{
> + free(sc->sc_rpc_buf, M_DEVBUF, sc->sc_rpc_buflen);
> +
> + sc->sc_rpc_buflen = len / VMT_RPC_BUFLEN * VMT_RPC_BUFLEN;
> + sc->sc_rpc_buflen += ((len % VMT_RPC_BUFLEN) <= 0) ? 0 :
> VMT_RPC_BUFLEN;
> + sc->sc_rpc_buf = malloc(sc->sc_rpc_buflen, M_DEVBUF, M_NOWAIT);
> ...
> + else if (srclen > SIZE_T_MAX)
> return (ENAMETOOLONG);
>
> *dstp = dst = malloc(srclen + 1, M_TEMP|M_ZERO, M_WAITOK);
>
> Userland may not ask the kernel to allocate such a huge object. The
> kernel address space is quite small. First off, huge allocations will
> fail.
>
> But it is worse -- the small kernel address space is shared for many
> purposes, so large allocations will harm the other subsystems.
>
> As written, this diff is too dangerous. Arbitrary allocation inside
> the kernel is not reasonable. object sizes requested by userland must
> be small, or the operations must be cut up, which does create impact
> on atomicity or other things.
As you pointed out, it is not a good idea to allocate large spaces
in kernel.
Would it be better to keep the current fixed length?
Prepare a variable like kern.maxpvbus and default it to
4096. Futhermore, how about free() after copyout() to user space?
Thank you.
--
ASOU Masato
Re: hostctl: Change from fixed length to variable length
Looking at these pieces:
+ sc->sc_rpc_buf = malloc(sc->sc_rpc_buflen, M_DEVBUF, M_NOWAIT);
...
+vm_rpc_buf_realloc(struct vmt_softc *sc, size_t len)
+{
+ free(sc->sc_rpc_buf, M_DEVBUF, sc->sc_rpc_buflen);
+
+ sc->sc_rpc_buflen = len / VMT_RPC_BUFLEN * VMT_RPC_BUFLEN;
+ sc->sc_rpc_buflen += ((len % VMT_RPC_BUFLEN) <= 0) ? 0 : VMT_RPC_BUFLEN;
+ sc->sc_rpc_buf = malloc(sc->sc_rpc_buflen, M_DEVBUF, M_NOWAIT);
...
+ else if (srclen > SIZE_T_MAX)
return (ENAMETOOLONG);
*dstp = dst = malloc(srclen + 1, M_TEMP|M_ZERO, M_WAITOK);
Userland may not ask the kernel to allocate such a huge object. The
kernel address space is quite small. First off, huge allocations will
fail.
But it is worse -- the small kernel address space is shared for many
purposes, so large allocations will harm the other subsystems.
As written, this diff is too dangerous. Arbitrary allocation inside
the kernel is not reasonable. object sizes requested by userland must
be small, or the operations must be cut up, which does create impact
on atomicity or other things.
hostctl: Change from fixed length to variable length
Hi,
The current VALUE length limit for the hostctl command for VMware is
fixed 4096 bytes. I want to pass a longer VALUE. I made a patch to
change it to variable length.
commane, ok?
Index: share/man/man4/pvbus.4
===
RCS file: /cvs/src/share/man/man4/pvbus.4,v
retrieving revision 1.14
diff -u -p -r1.14 pvbus.4
--- share/man/man4/pvbus.4 14 Jun 2017 12:42:09 - 1.14
+++ share/man/man4/pvbus.4 5 Oct 2022 03:23:00 -
@@ -102,13 +102,15 @@ struct pvbus_req {
char*pvr_key;
size_t pvr_valuelen;
char*pvr_value;
+ size_t *pvr_len;
};
.Ed
.Pp
The caller is responsible for attaching character buffers to the
-.Fa pvr_key
-and
+.Fa pvr_key ,
.Fa pvr_value
+and
+.Fa pvr_len
fields and to set their length in
.Fa pvr_keylen
and
@@ -136,6 +138,14 @@ Write the new value
to the key
.Fa pvr_key .
This command requires write permissions on the device file.
+.It Dv PVBUSIOC_GETVALUELEN
+Get the length of the value of
+.Fa pvr_key
+and return it in
+.Fa pvr_len .
+.It Dv PVBUSIOC_GETTYPELEN
+Get the length of the type and return it in
+.Fa pvr_len .
.El
.Sh FILES
.Bl -tag -width "/dev/pvbusX" -compact
Index: sys/dev/pv/hypervic.c
===
RCS file: /cvs/src/sys/dev/pv/hypervic.c,v
retrieving revision 1.17
diff -u -p -r1.17 hypervic.c
--- sys/dev/pv/hypervic.c 8 Sep 2022 10:22:06 - 1.17
+++ sys/dev/pv/hypervic.c 5 Oct 2022 03:23:01 -
@@ -105,7 +105,7 @@ int hv_heartbeat_attach(struct hv_ic_dev
void hv_heartbeat(void *);
inthv_kvp_attach(struct hv_ic_dev *);
void hv_kvp(void *);
-inthv_kvop(void *, int, char *, char *, size_t);
+inthv_kvop(void *, int, char *, size_t, char *, size_t, size_t *);
inthv_shutdown_attach(struct hv_ic_dev *);
void hv_shutdown(void *);
inthv_timesync_attach(struct hv_ic_dev *);
@@ -551,11 +551,10 @@ kvp_pool_init(struct kvp_pool *kvpl)
}
static int
-kvp_pool_insert(struct kvp_pool *kvpl, const char *key, const char *val,
-uint32_t vallen, uint32_t valtype)
+kvp_pool_insert(struct kvp_pool *kvpl, const char *key, size_t keylen,
+const char *val, size_t vallen, uint32_t valtype)
{
struct kvp_entry *kpe;
- int keylen = strlen(key);
if (keylen > HV_KVP_MAX_KEY_SIZE / 2)
return (ERANGE);
@@ -592,11 +591,10 @@ kvp_pool_insert(struct kvp_pool *kvpl, c
}
static int
-kvp_pool_update(struct kvp_pool *kvpl, const char *key, const char *val,
-uint32_t vallen, uint32_t valtype)
+kvp_pool_update(struct kvp_pool *kvpl, const char *key, size_t keylen,
+const char *val, size_t vallen, uint32_t valtype)
{
struct kvp_entry *kpe;
- int keylen = strlen(key);
if (keylen > HV_KVP_MAX_KEY_SIZE / 2)
return (ERANGE);
@@ -715,12 +713,13 @@ kvp_pool_remove(struct kvp_pool *kvpl, c
}
static int
-kvp_pool_extract(struct kvp_pool *kvpl, const char *key, char *val,
-uint32_t vallen)
+kvp_pool_extract(struct kvp_pool *kvpl, int op, const char *key, char *val,
+size_t vallen, size_t *vlen)
{
struct kvp_entry *kpe;
+ char buf[128];
- if (vallen < HV_KVP_MAX_VAL_SIZE / 2)
+ if (vallen > SIZE_T_MAX)
return (ERANGE);
mtx_enter(>kvp_lock);
@@ -734,18 +733,36 @@ kvp_pool_extract(struct kvp_pool *kvpl,
return (ENOENT);
}
- switch (kpe->kpe_valtype) {
- case HV_KVP_REG_SZ:
- strlcpy(val, kpe->kpe_val, HV_KVP_MAX_VAL_SIZE / 2);
- break;
- case HV_KVP_REG_U32:
- snprintf(val, HV_KVP_MAX_VAL_SIZE / 2, "%u",
- *(uint32_t *)kpe->kpe_val);
- break;
- case HV_KVP_REG_U64:
- snprintf(val, HV_KVP_MAX_VAL_SIZE / 2, "%llu",
- *(uint64_t *)kpe->kpe_val);
- break;
+ if (op == PVBUS_KVGETVALLEN) {
+ switch (kpe->kpe_valtype) {
+ case HV_KVP_REG_SZ:
+ *vlen = strlen(kpe->kpe_val);
+ break;
+ case HV_KVP_REG_U32:
+ snprintf(buf, HV_KVP_MAX_VAL_SIZE / 2, "%u",
+ *(uint32_t *)kpe->kpe_val);
+ *vlen = strlen(buf);
+ break;
+ case HV_KVP_REG_U64:
+ snprintf(buf, HV_KVP_MAX_VAL_SIZE / 2, "%llu",
+ *(uint64_t *)kpe->kpe_val);
+ *vlen = strlen(buf);
+ break;
+ }
+ } else {
+ switch (kpe->kpe_valtype) {
+ case HV_KVP_REG_SZ:
+ strlcpy(val, kpe->kpe_val, HV_KVP_MAX_VAL_SIZE / 2);
+ break;
+ case HV_KVP_REG_U32:
+
