Hi, On Mon, 26 Dec 2022 13:37:45 +0900 (JST) Masato Asou <[email protected]> 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(&value[pos], iovp[i].iov_base, valuelen > - pos); > } > xs_resfree(&xst, iovp, iov_cnt); > break;
