Re: xenstore.c: return error number

2022-11-08 Thread Masato Asou
From: Martin Pieuchot 
Date: Tue, 8 Nov 2022 11:12:43 +

> On 01/11/22(Tue) 15:26, Masato Asou wrote:
>> Hi,
>> 
>> Return error number instead of call panic().
> 
> Makes sense to me.  Do you know how this error can occur?  Is is a logic
> error or are we trusting values produced by a third party?

You can reproduce this error by writing 1025 bytes from Dom0 and
reading this from OpenBSD on the DomU as follows:

dom0$ sudo xenstore-write /vm/408d6e98-1b95-43c2-803b-de24ee205631/data 
012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789abcde

domu$ hostctl /vm/408d6e98-1b95-43c2-803b-de24ee205631/data

--
ASOU Masato

>> comment, ok?
>> --
>> ASOU Masato
>> 
>> diff --git a/sys/dev/pv/xenstore.c b/sys/dev/pv/xenstore.c
>> index 1e4f15d30eb..dc89ba0fa6d 100644
>> --- a/sys/dev/pv/xenstore.c
>> +++ b/sys/dev/pv/xenstore.c
>> @@ -118,6 +118,7 @@ struct xs_msg {
>>  struct xs_msghdr xsm_hdr;
>>  uint32_t xsm_read;
>>  uint32_t xsm_dlen;
>> +int  xsm_error;
>>  uint8_t *xsm_data;
>>  TAILQ_ENTRY(xs_msg)  xsm_link;
>>  };
>> @@ -566,9 +567,7 @@ xs_intr(void *arg)
>>  }
>>  
>>  if (xsm->xsm_hdr.xmh_len > xsm->xsm_dlen)
>> -panic("message too large: %d vs %d for type %d, rid %u",
>> -xsm->xsm_hdr.xmh_len, xsm->xsm_dlen, xsm->xsm_hdr.xmh_type,
>> -xsm->xsm_hdr.xmh_rid);
>> +xsm->xsm_error = EMSGSIZE;
>>  
>>  len = MIN(xsm->xsm_hdr.xmh_len - xsm->xsm_read, avail);
>>  if (len) {
>> @@ -800,7 +799,9 @@ xs_cmd(struct xs_transaction *xst, int cmd, const char 
>> *path,
>>  error = xs_geterror(xsm);
>>  DPRINTF("%s: xenstore request %d \"%s\" error %s\n",
>>  xs->xs_sc->sc_dev.dv_xname, cmd, path, xsm->xsm_data);
>> -} else if (mode == READ) {
>> +} else if (xsm->xsm_error != 0)
>> +error = xsm->xsm_error;
>> +else if (mode == READ) {
>>  KASSERT(iov && iov_cnt);
>>  error = xs_parse(xst, xsm, iov, iov_cnt);
>>  }
>> 


Re: xenstore.c: return error number

2022-11-08 Thread Martin Pieuchot
On 01/11/22(Tue) 15:26, Masato Asou wrote:
> Hi,
> 
> Return error number instead of call panic().

Makes sense to me.  Do you know how this error can occur?  Is is a logic
error or are we trusting values produced by a third party?

> comment, ok?
> --
> ASOU Masato
> 
> diff --git a/sys/dev/pv/xenstore.c b/sys/dev/pv/xenstore.c
> index 1e4f15d30eb..dc89ba0fa6d 100644
> --- a/sys/dev/pv/xenstore.c
> +++ b/sys/dev/pv/xenstore.c
> @@ -118,6 +118,7 @@ struct xs_msg {
>   struct xs_msghdr xsm_hdr;
>   uint32_t xsm_read;
>   uint32_t xsm_dlen;
> + int  xsm_error;
>   uint8_t *xsm_data;
>   TAILQ_ENTRY(xs_msg)  xsm_link;
>  };
> @@ -566,9 +567,7 @@ xs_intr(void *arg)
>   }
>  
>   if (xsm->xsm_hdr.xmh_len > xsm->xsm_dlen)
> - panic("message too large: %d vs %d for type %d, rid %u",
> - xsm->xsm_hdr.xmh_len, xsm->xsm_dlen, xsm->xsm_hdr.xmh_type,
> - xsm->xsm_hdr.xmh_rid);
> + xsm->xsm_error = EMSGSIZE;
>  
>   len = MIN(xsm->xsm_hdr.xmh_len - xsm->xsm_read, avail);
>   if (len) {
> @@ -800,7 +799,9 @@ xs_cmd(struct xs_transaction *xst, int cmd, const char 
> *path,
>   error = xs_geterror(xsm);
>   DPRINTF("%s: xenstore request %d \"%s\" error %s\n",
>   xs->xs_sc->sc_dev.dv_xname, cmd, path, xsm->xsm_data);
> - } else if (mode == READ) {
> + } else if (xsm->xsm_error != 0)
> + error = xsm->xsm_error;
> + else if (mode == READ) {
>   KASSERT(iov && iov_cnt);
>   error = xs_parse(xst, xsm, iov, iov_cnt);
>   }
> 



xenstore.c: return error number

2022-10-31 Thread Masato Asou
Hi,

Return error number instead of call panic().

comment, ok?
--
ASOU Masato

diff --git a/sys/dev/pv/xenstore.c b/sys/dev/pv/xenstore.c
index 1e4f15d30eb..dc89ba0fa6d 100644
--- a/sys/dev/pv/xenstore.c
+++ b/sys/dev/pv/xenstore.c
@@ -118,6 +118,7 @@ struct xs_msg {
struct xs_msghdr xsm_hdr;
uint32_t xsm_read;
uint32_t xsm_dlen;
+   int  xsm_error;
uint8_t *xsm_data;
TAILQ_ENTRY(xs_msg)  xsm_link;
 };
@@ -566,9 +567,7 @@ xs_intr(void *arg)
}
 
if (xsm->xsm_hdr.xmh_len > xsm->xsm_dlen)
-   panic("message too large: %d vs %d for type %d, rid %u",
-   xsm->xsm_hdr.xmh_len, xsm->xsm_dlen, xsm->xsm_hdr.xmh_type,
-   xsm->xsm_hdr.xmh_rid);
+   xsm->xsm_error = EMSGSIZE;
 
len = MIN(xsm->xsm_hdr.xmh_len - xsm->xsm_read, avail);
if (len) {
@@ -800,7 +799,9 @@ xs_cmd(struct xs_transaction *xst, int cmd, const char 
*path,
error = xs_geterror(xsm);
DPRINTF("%s: xenstore request %d \"%s\" error %s\n",
xs->xs_sc->sc_dev.dv_xname, cmd, path, xsm->xsm_data);
-   } else if (mode == READ) {
+   } else if (xsm->xsm_error != 0)
+   error = xsm->xsm_error;
+   else if (mode == READ) {
KASSERT(iov && iov_cnt);
error = xs_parse(xst, xsm, iov, iov_cnt);
}