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)) {
                /*

Reply via email to