ok yasuoka
On Fri, 06 Jan 2023 15:14:05 +0900 (JST)
Masato Asou <a...@soum.co.jp> wrote:
> I have updated my patch.
>
> From: YASUOKA Masahiko <yasu...@openbsd.org>
> 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 <yasu...@openbsd.org>
> 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(&xst, 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 -0000 1.14
> +++ share/man/man4/pvbus.4 5 Jan 2023 23:20:39 -0000
> @@ -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 -0000 1.17
> +++ sys/dev/pv/hypervic.c 5 Jan 2023 23:20:40 -0000
> @@ -1151,11 +1151,12 @@ hv_kvop(void *arg, int op, char *key, ch
> kvpl = &kvp->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, &keylen))
> + if (val + vallen < vp + HV_KVP_MAX_KEY_SIZE / 2)
> + return (ERANGE);
> + if (kvp_pool_keys(kvpl, next, vp, &keylen))
> 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 -0000 1.26
> +++ sys/dev/pv/pvbus.c 5 Jan 2023 23:20:40 -0000
> @@ -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
> retrieving revision 1.10
> diff -u -p -r1.10 pvvar.h
> --- sys/dev/pv/pvvar.h 22 Jun 2017 06:21:12 -0000 1.10
> +++ sys/dev/pv/pvvar.h 5 Jan 2023 23:20:40 -0000
> @@ -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)
>
> +#define PVBUS_KVOP_MAXSIZE (64 * 1024)
> +
> #ifdef _KERNEL
> enum {
> PVBUS_KVM,
> Index: sys/dev/pv/vmt.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pv/vmt.c,v
> retrieving revision 1.29
> diff -u -p -r1.29 vmt.c
> --- sys/dev/pv/vmt.c 28 Dec 2022 10:11:36 -0000 1.29
> +++ sys/dev/pv/vmt.c 5 Jan 2023 23:20:40 -0000
> @@ -547,7 +547,7 @@ vmt_kvop(void *arg, int op, char *key, c
>
> if (rlen > 0) {
> if (rlen + 1 > valuelen) {
> - error = EMSGSIZE;
> + error = ERANGE;
> goto close;
> }
>
> Index: sys/dev/pv/xenstore.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pv/xenstore.c,v
> retrieving revision 1.47
> diff -u -p -r1.47 xenstore.c
> --- sys/dev/pv/xenstore.c 10 Nov 2022 02:47:52 -0000 1.47
> +++ sys/dev/pv/xenstore.c 5 Jan 2023 23:20:40 -0000
> @@ -1116,11 +1116,16 @@ xs_kvop(void *xsc, int op, char *key, ch
> /* 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(&xst, iovp, iov_cnt);
> break;
> @@ -1128,5 +1133,5 @@ xs_kvop(void *xsc, int op, char *key, ch
> break;
> }
>
> - return (0);
> + return (error);
> }
> Index: usr.sbin/hostctl/hostctl.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/hostctl/hostctl.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 hostctl.c
> --- usr.sbin/hostctl/hostctl.c 28 Jun 2019 13:32:47 -0000 1.5
> +++ usr.sbin/hostctl/hostctl.c 5 Jan 2023 23:20:41 -0000
> @@ -178,14 +178,28 @@ main(int argc, char *argv[])
> usage();
>
> /* Re-open read-writable */
> - if (cmd == PVBUSIOC_KVWRITE) {
> + if (cmd != PVBUSIOC_KVREAD) {
> close(fd);
> if ((fd = open(path_pvbus, O_RDWR)) == -1)
> err(1, "open: %s", path_pvbus);
> + if ((ret = ioctl(fd, cmd, &pvr, sizeof(pvr))) == -1)
> + err(1, "ioctl");
> + } else {
> + while (1) {
> + if ((ret = ioctl(fd, cmd, &pvr, sizeof(pvr))) == 0)
> + break;
> + if (errno == ERANGE &&
> + pvr.pvr_valuelen < PVBUS_KVOP_MAXSIZE) {
> + /* the buffer is not enough, expand it */
> + pvr.pvr_valuelen *= 2;
> + if ((pvr.pvr_value = realloc(pvr.pvr_value,
> + pvr.pvr_valuelen)) == NULL)
> + err(1, "realloc");
> + continue;
> + }
> + err(1, "ioctl");
> + }
> }
> -
> - if ((ret = ioctl(fd, cmd, &pvr, sizeof(pvr))) == -1)
> - err(1, "ioctl");
>
> if (!qflag && strlen(pvr.pvr_value)) {
> /*
>
