Re: [RFC PATCH v3 2/4] vsock/vmci: convert VMCI error code to -ENOMEM on receive

2023-03-30 Thread Vishnu Dasa via Virtualization



> On Mar 30, 2023, at 1:18 PM, Arseniy Krasnov  wrote:
> 
> !! External Email
> 
> On 30.03.2023 23:13, Arseniy Krasnov wrote:
>> This adds conversion of VMCI specific error code to general -ENOMEM. It
>> is needed, because af_vsock.c passes error value returned from transport
>> to the user, which does not expect to get VMCI_ERROR_* values.
> 
> @Stefano, I have some doubts about this commit message, as it says "... 
> af_vsock.c
> passes error value returned from transport to the user ...", but this
> behaviour is implemented only in the next patch. Is it ok, if both patches
> are in a single patchset?
> 
> For patch 1 I think it is ok, as it fixes current implementation.
> 
> Thanks, Arseniy
> 
>> 
>> Signed-off-by: Arseniy Krasnov 

Code change looks good to me.  Will let you figure out the commit message
with Stefano. Thanks!

Reviewed-by: Vishnu Dasa 

>> ---
>> net/vmw_vsock/vmci_transport.c | 11 +--
>> 1 file changed, 9 insertions(+), 2 deletions(-)
>> 
>> diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
>> index 95cc4d79ba29..b370070194fa 100644
>> --- a/net/vmw_vsock/vmci_transport.c
>> +++ b/net/vmw_vsock/vmci_transport.c
>> @@ -1831,10 +1831,17 @@ static ssize_t vmci_transport_stream_dequeue(
>>  size_t len,
>>  int flags)
>> {
>> + ssize_t err;
>> +
>>  if (flags & MSG_PEEK)
>> - return vmci_qpair_peekv(vmci_trans(vsk)->qpair, msg, len, 0);
>> + err = vmci_qpair_peekv(vmci_trans(vsk)->qpair, msg, len, 0);
>>  else
>> - return vmci_qpair_dequev(vmci_trans(vsk)->qpair, msg, len, 0);
>> + err = vmci_qpair_dequev(vmci_trans(vsk)->qpair, msg, len, 0);
>> +
>> + if (err < 0)
>> + err = -ENOMEM;
>> +
>> + return err;
>> }
>> 
>> static ssize_t vmci_transport_stream_enqueue(
> 
> !! External Email: This email originated from outside of the organization. Do 
> not click links or open attachments unless you recognize the sender.

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [RFC PATCH v3 1/4] vsock/vmci: convert VMCI error code to -ENOMEM on send

2023-03-30 Thread Vishnu Dasa via Virtualization



> On Mar 30, 2023, at 1:12 PM, Arseniy Krasnov  wrote:
> 
> !! External Email
> 
> This adds conversion of VMCI specific error code to general -ENOMEM. It
> is needed, because af_vsock.c passes error value returned from transport
> to the user, which does not expect to get VMCI_ERROR_* values.
> 
> Fixes: c43170b7e157 ("vsock: return errors other than -ENOMEM to socket")
> Signed-off-by: Arseniy Krasnov 

Thanks, looks good to me.

Reviewed-by: Vishnu Dasa 

> ---
> net/vmw_vsock/vmci_transport.c | 8 +++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
> index 36eb16a40745..95cc4d79ba29 100644
> --- a/net/vmw_vsock/vmci_transport.c
> +++ b/net/vmw_vsock/vmci_transport.c
> @@ -1842,7 +1842,13 @@ static ssize_t vmci_transport_stream_enqueue(
>struct msghdr *msg,
>size_t len)
> {
> -   return vmci_qpair_enquev(vmci_trans(vsk)->qpair, msg, len, 0);
> +   ssize_t err;
> +
> +   err = vmci_qpair_enquev(vmci_trans(vsk)->qpair, msg, len, 0);
> +   if (err < 0)
> +   err = -ENOMEM;
> +
> +   return err;
> }
> 
> static s64 vmci_transport_stream_has_data(struct vsock_sock *vsk)
> --
> 2.25.1
> 
> !! External Email: This email originated from outside of the organization. Do 
> not click links or open attachments unless you recognize the sender.

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [RFC PATCH v2 2/3] vsock/vmci: convert VMCI error code to -ENOMEM

2023-03-30 Thread Vishnu Dasa via Virtualization



> On Mar 30, 2023, at 1:19 AM, Stefano Garzarella  wrote:
> 
> !! External Email
> 
> On Thu, Mar 30, 2023 at 10:07:36AM +0300, Arseniy Krasnov wrote:
>> This adds conversion of VMCI specific error code to general -ENOMEM. It
>> is needed, because af_vsock.c passes error value returned from transport
>> to the user.
>> 
>> Signed-off-by: Arseniy Krasnov 
>> ---
>> net/vmw_vsock/vmci_transport.c | 19 ---
>> 1 file changed, 16 insertions(+), 3 deletions(-)
>> 
>> diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
>> index 36eb16a40745..45de3e75597f 100644
>> --- a/net/vmw_vsock/vmci_transport.c
>> +++ b/net/vmw_vsock/vmci_transport.c
>> @@ -1831,10 +1831,17 @@ static ssize_t vmci_transport_stream_dequeue(
>>  size_t len,
>>  int flags)
>> {
>> +  int err;
> 
> Please, use the same type returned by the function.
> 
>> +
>>  if (flags & MSG_PEEK)
>> -  return vmci_qpair_peekv(vmci_trans(vsk)->qpair, msg, len, 0);
>> +  err = vmci_qpair_peekv(vmci_trans(vsk)->qpair, msg, len, 0);
>>  else
>> -  return vmci_qpair_dequev(vmci_trans(vsk)->qpair, msg, len, 0);
>> +  err = vmci_qpair_dequev(vmci_trans(vsk)->qpair, msg, len, 0);
>> +
>> +  if (err < 0)
>> +  err = -ENOMEM;
>> +
>> +  return err;
>> }
>> 
>> static ssize_t vmci_transport_stream_enqueue(
>> @@ -1842,7 +1849,13 @@ static ssize_t vmci_transport_stream_enqueue(
>>  struct msghdr *msg,
>>  size_t len)
>> {
>> -  return vmci_qpair_enquev(vmci_trans(vsk)->qpair, msg, len, 0);
>> +  int err;
> 
> Ditto.
> 
>> +
>> +  err = vmci_qpair_enquev(vmci_trans(vsk)->qpair, msg, len, 0);
>> +  if (err < 0)
>> +  err = -ENOMEM;
>> +
>> +  return err;
>> }
> 
> @Vishnu: should we backport the change for
> vmci_transport_stream_enqueue() to stable branches?
> 
> In this case I would split this patch and I would send the
> vmci_transport_stream_enqueue() change to the net branch including:
> 
> Fixes: c43170b7e157 ("vsock: return errors other than -ENOMEM to socket")

Yes, good point.  It would be better to do it this way for correctness.

Thanks,
Vishnu

> 
> Thanks,
> Stefano
> 
> 
> !! External Email: This email originated from outside of the organization. Do 
> not click links or open attachments unless you recognize the sender.


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [RFC PATCH v1 1/2] vsock: return errors other than -ENOMEM to socket

2023-03-29 Thread Vishnu Dasa via Virtualization


> On Mar 28, 2023, at 4:20 AM, Arseniy Krasnov  wrote:
> 
> !! External Email
> 
> On 28.03.2023 14:19, Stefano Garzarella wrote:
>> On Tue, Mar 28, 2023 at 01:42:19PM +0300, Arseniy Krasnov wrote:
>>> 
>>> 
>>> On 28.03.2023 12:42, Stefano Garzarella wrote:
 I pressed send too early...
 
 CCing Bryan, Vishnu, and pv-driv...@vmware.com
 
 On Tue, Mar 28, 2023 at 11:39 AM Stefano Garzarella  
 wrote:
> 
> On Sun, Mar 26, 2023 at 01:13:11AM +0300, Arseniy Krasnov wrote:
>> This removes behaviour, where error code returned from any transport
>> was always switched to ENOMEM. This works in the same way as:
>> commit
>> c43170b7e157 ("vsock: return errors other than -ENOMEM to socket"),
>> but for receive calls.
>> 
>> Signed-off-by: Arseniy Krasnov 
>> ---
>> net/vmw_vsock/af_vsock.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>> index 19aea7cba26e..9262e0b77d47 100644
>> --- a/net/vmw_vsock/af_vsock.c
>> +++ b/net/vmw_vsock/af_vsock.c
>> @@ -2007,7 +2007,7 @@ static int __vsock_stream_recvmsg(struct sock *sk, 
>> struct msghdr *msg,
>> 
>>  read = transport->stream_dequeue(vsk, msg, len - copied, 
>> flags);
> 
> In vmci_transport_stream_dequeue() vmci_qpair_peekv() and
> vmci_qpair_dequev() return VMCI_ERROR_* in case of errors.
> 
> Maybe we should return -ENOMEM in vmci_transport_stream_dequeue() if
> those functions fail to keep the same behavior.
>>> 
>>> Yes, seems i missed it, because several months ago we had similar question 
>>> for send
>>> logic:
>>> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.spinics.net%2Flists%2Fkernel%2Fmsg4611091.html=05%7C01%7Cvdasa%40vmware.com%7C3b17793425384debe75708db2f7eec8c%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C638155994413494900%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C=MMfFcKuFFvMcJrbToKvWvIB%2FZmzp%2BdGGVWFVWztuSzg%3D=0
>>> And it was ok to not handle VMCI send path in this way. So i think current 
>>> implementation
>>> for tx is a little bit buggy, because VMCI specific error from 
>>> 'vmci_qpair_enquev()' is
>>> returned to af_vsock.c. I think error conversion must be added to VMCI 
>>> transport for tx
>>> also.
>> 
>> Good point!
>> 
>> These are negative values, so there are no big problems, but I don't
>> know what the user expects in this case.
>> 
>> @Vishnu Do we want to return an errno to the user or a VMCI_ERROR_*?
> 
> Small remark, as i can see, VMCI_ERROR_ is not exported to user in 
> include/uapi,
> so IIUC user won't be able to interpret such values correctly.
> 
> Thanks, Arseniy

Let's just return -ENOMEM from vmci transport in case of error in
vmci_transport_stream_enqueue and vmci_transport_stream_dequeue.

@Arseniy,
Could you please add a separate patch in this set to handle the above?

Thanks,
Vishnu

> 
>> 
>> In both cases I think we should do the same for both enqueue and
>> dequeue.
>> 
>>> 
>>> Good thing is that Hyper-V uses general error codes.
>> 
>> Yeah!
>> 
>> Thanks,
>> Stefano
>> 
>>> 
>>> Thanks, Arseniy
> 
> CCing Bryan, Vishnu, and pv-driv...@vmware.com
> 
> The other transports seem okay to me.
> 
> Thanks,
> Stefano
> 
>>  if (read < 0) {
>> -  err = -ENOMEM;
>> +  err = read;
>>  break;
>>  }
>> 
>> @@ -2058,7 +2058,7 @@ static int __vsock_seqpacket_recvmsg(struct sock 
>> *sk, struct msghdr *msg,
>>  msg_len = transport->seqpacket_dequeue(vsk, msg, flags);
>> 
>>  if (msg_len < 0) {
>> -  err = -ENOMEM;
>> +  err = msg_len;
>>  goto out;
>>  }
>> 
>> --
>> 2.25.1
>> 
 
>>> 
>> 
> 
> !! External Email: This email originated from outside of the organization. Do 
> not click links or open attachments unless you recognize the sender.


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v3] net: vmw_vsock: vmci: Check memcpy_from_msg()

2022-12-05 Thread Vishnu Dasa via Virtualization


> On Dec 5, 2022, at 3:52 AM, Artem Chernyshev  
> wrote:
> 
> vmci_transport_dgram_enqueue() does not check the return value
> of memcpy_from_msg(). Return with an error if the memcpy fails.

I think we can add some more information in the description.  Sorry, I
should've said this earlier.

vmci_transport_dgram_enqueue() does not check the return value
of memcpy_from_msg().  If memcpy_from_msg() fails, it is possible that
uninitialized memory contents are sent unintentionally instead of user's
message in the datagram to the destination.  Return with an error if
memcpy_from_msg() fails.

> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Fixes: 0f7db23a07af ("vmci_transport: switch ->enqeue_dgram, ->enqueue_stream 
> and ->dequeue_stream to msghdr")
> Signed-off-by: Artem Chernyshev 

Thanks, Artem!  This version looks good to me modulo my suggestion
about the description above.

Reviewed-by: Vishnu Dasa 

Regards,
Vishnu

> ---
> V1->V2 Fix memory leaking and updates for description
> V2->V3 Return the value of memcpy_from_msg()
> 
> net/vmw_vsock/vmci_transport.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
> index 842c94286d31..36eb16a40745 100644
> --- a/net/vmw_vsock/vmci_transport.c
> +++ b/net/vmw_vsock/vmci_transport.c
> @@ -1711,7 +1711,11 @@ static int vmci_transport_dgram_enqueue(
>   if (!dg)
>   return -ENOMEM;
> 
> - memcpy_from_msg(VMCI_DG_PAYLOAD(dg), msg, len);
> + err = memcpy_from_msg(VMCI_DG_PAYLOAD(dg), msg, len);
> + if (err) {
> + kfree(dg);
> + return err;
> + }
> 
>   dg->dst = vmci_make_handle(remote_addr->svm_cid,
>  remote_addr->svm_port);
> -- 
> 2.30.3
> 

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH] net: vmw_vsock: vmci: Check memcpy_from_msg()

2022-12-02 Thread Vishnu Dasa via Virtualization



> On Dec 2, 2022, at 2:58 PM, Artem Chernyshev  
> wrote:
> 
> We returns from vmci_transport_dgram_enqueue() with error
> if memcpy goes wrong

Thanks for the patch.

Nit: could you please update the description?  Maybe something like -
vmci_transport_dgram_enqueue() does not check the return value
of memcpy_from_msg(). Return with an error if the memcpy fails.

> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Fixes: 0f7db23a07af ("vmci_transport: switch ->enqeue_dgram, ->enqueue_stream 
> and ->dequeue_stream to msghdr")
> Signed-off-by: Artem Chernyshev 
> ---
> net/vmw_vsock/vmci_transport.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
> index 842c94286d31..7994090e0314 100644
> --- a/net/vmw_vsock/vmci_transport.c
> +++ b/net/vmw_vsock/vmci_transport.c
> @@ -1711,7 +1711,8 @@ static int vmci_transport_dgram_enqueue(
>   if (!dg)
>   return -ENOMEM;
> 
> - memcpy_from_msg(VMCI_DG_PAYLOAD(dg), msg, len);
> + if (memcpy_from_msg(VMCI_DG_PAYLOAD(dg), msg, len))

Need to free dg using kfree() before returning.

> + return -EFAULT;
> 
>   dg->dst = vmci_make_handle(remote_addr->svm_cid,
>  remote_addr->svm_port);
> -- 
> 2.30.3
> 

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [RFC PATCH v2 3/6] vsock/vmci: always return ENOMEM in case of error

2022-12-01 Thread Vishnu Dasa via Virtualization



> On Dec 1, 2022, at 1:30 AM, Stefano Garzarella  wrote:
> 
> !! External Email
> 
> On Fri, Nov 25, 2022 at 05:08:06PM +, Arseniy Krasnov wrote:
>> From: Bobby Eshleman 
>> 
>> This saves original behaviour from af_vsock.c - switch any error
>> code returned from transport layer to ENOMEM.
>> 
>> Signed-off-by: Bobby Eshleman 
>> Signed-off-by: Arseniy Krasnov 
>> ---
>> net/vmw_vsock/vmci_transport.c | 9 -
>> 1 file changed, 8 insertions(+), 1 deletion(-)
> 
> @Bryan @Vishnu what do you think about this patch?
> 
> A bit of context:
> 
> Before this series, the af_vsock core always returned ENOMEM to the user
> if the transport failed to queue the packet.
> 
> Now we are changing it by returning the transport error. So I think here
> we want to preserve the previous behavior for vmci, but I don't know if
> that's the right thing.
> 

Agree with Stefano.  I don't think we need to preserve the previous
behavior for vmci.

> 
> @Arseniy please in the next versions describe better in the commit
> messages the reasons for these changes, so it is easier review for
> others and also in the future by reading the commit message we can
> understand the reason for the change.
> 
> Thanks,
> Stefano
> 
>> 
>> diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
>> index 842c94286d31..289a36a203a2 100644
>> --- a/net/vmw_vsock/vmci_transport.c
>> +++ b/net/vmw_vsock/vmci_transport.c
>> @@ -1838,7 +1838,14 @@ static ssize_t vmci_transport_stream_enqueue(
>>  struct msghdr *msg,
>>  size_t len)
>> {
>> -  return vmci_qpair_enquev(vmci_trans(vsk)->qpair, msg, len, 0);
>> +  int err;
>> +
>> +  err = vmci_qpair_enquev(vmci_trans(vsk)->qpair, msg, len, 0);
>> +
>> +  if (err < 0)
>> +  err = -ENOMEM;
>> +
>> +  return err;
>> }
>> 
>> static s64 vmci_transport_stream_has_data(struct vsock_sock *vsk)
>> --
>> 2.25.1
> 
> 
> !! External Email: This email originated from outside of the organization. Do 
> not click links or open attachments unless you recognize the sender.

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH 0/3] MAINTAINERS: Update entries for some VMware drivers

2022-09-10 Thread Vishnu Dasa via Virtualization


> On Sep 7, 2022, at 12:51 AM, Stefano Garzarella  wrote:
> 
> On Tue, Sep 06, 2022 at 10:27:19AM -0700, vd...@vmware.com wrote:
>> From: Vishnu Dasa 
>> 
>> This series updates a few existing maintainer entries for VMware
>> supported drivers and adds a new entry for vsock vmci transport
>> driver.
>> 
> 
> Since you are updating MAINTAINERS, what about adding 
> "include/linux/vmw_vmci*" under "VMWARE VMCI DRIVER"?

I'll send a separate patch for this.  Thanks!!

> Thanks,
> Stefano
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [RFC PATCH v3 4/9] vmci/vsock: use 'target' in notify_poll_in callback

2022-08-18 Thread Vishnu Dasa via Virtualization


> On Aug 8, 2022, at 3:36 AM, Stefano Garzarella  wrote:
> 
> On Wed, Aug 03, 2022 at 01:57:54PM +, Arseniy Krasnov wrote:
>> This callback controls setting of POLLIN,POLLRDNORM output bits of poll()
>> syscall,but in some cases,it is incorrectly to set it, when socket has
>> at least 1 bytes of available data. Use 'target' which is already exists
>> and equal to sk_rcvlowat in this case.
> 
> Ditto as the previous patch.
> With that fixed:
> 
> Reviewed-by: Stefano Garzarella 
> 
> @Bryan, @Vishnu, if you're happy with this change, can you ack/review?

This patch looks good to me.

Thank you, Arseniy for running the test with VMCI.  I also ran some of
our internal tests successfully with this patch series.

> Thanks,
> Stefano
> 
>> 
>> Signed-off-by: Arseniy Krasnov 

Reviewed-by: Vishnu Dasa 

>> ---
>> net/vmw_vsock/vmci_transport_notify.c| 8 
>> net/vmw_vsock/vmci_transport_notify_qstate.c | 8 
>> 2 files changed, 8 insertions(+), 8 deletions(-)
>> 
>> diff --git a/net/vmw_vsock/vmci_transport_notify.c 
>> b/net/vmw_vsock/vmci_transport_notify.c
>> index d69fc4b595ad..852097e2b9e6 100644
>> --- a/net/vmw_vsock/vmci_transport_notify.c
>> +++ b/net/vmw_vsock/vmci_transport_notify.c
>> @@ -340,12 +340,12 @@ vmci_transport_notify_pkt_poll_in(struct sock *sk,
>> {
>>  struct vsock_sock *vsk = vsock_sk(sk);
>> 
>> -  if (vsock_stream_has_data(vsk)) {
>> +  if (vsock_stream_has_data(vsk) >= target) {
>>  *data_ready_now = true;
>>  } else {
>> -  /* We can't read right now because there is nothing in the
>> -   * queue. Ask for notifications when there is something to
>> -   * read.
>> +  /* We can't read right now because there is not enough data
>> +   * in the queue. Ask for notifications when there is something
>> +   * to read.
>>   */
>>  if (sk->sk_state == TCP_ESTABLISHED) {
>>  if (!send_waiting_read(sk, 1))
>> diff --git a/net/vmw_vsock/vmci_transport_notify_qstate.c 
>> b/net/vmw_vsock/vmci_transport_notify_qstate.c
>> index 0f36d7c45db3..12f0cb8fe998 100644
>> --- a/net/vmw_vsock/vmci_transport_notify_qstate.c
>> +++ b/net/vmw_vsock/vmci_transport_notify_qstate.c
>> @@ -161,12 +161,12 @@ vmci_transport_notify_pkt_poll_in(struct sock *sk,
>> {
>>  struct vsock_sock *vsk = vsock_sk(sk);
>> 
>> -  if (vsock_stream_has_data(vsk)) {
>> +  if (vsock_stream_has_data(vsk) >= target) {
>>  *data_ready_now = true;
>>  } else {
>> -  /* We can't read right now because there is nothing in the
>> -   * queue. Ask for notifications when there is something to
>> -   * read.
>> +  /* We can't read right now because there is not enough data
>> +   * in the queue. Ask for notifications when there is something
>> +   * to read.
>>   */
>>  if (sk->sk_state == TCP_ESTABLISHED)
>>  vsock_block_update_write_window(sk);
>> --
>> 2.25.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [RFC PATCH v3 8/9] vmci/vsock: check SO_RCVLOWAT before wake up reader

2022-08-18 Thread Vishnu Dasa via Virtualization



> On Aug 8, 2022, at 4:01 AM, Stefano Garzarella  wrote:
> 
> On Wed, Aug 03, 2022 at 02:05:52PM +, Arseniy Krasnov wrote:
>> This adds extra condition to wake up data reader: do it only when number
>> of readable bytes >= SO_RCVLOWAT. Otherwise, there is no sense to kick
>> user,because it will wait until SO_RCVLOWAT bytes will be dequeued.

Nit: add space after comma.

> Ditto as previous patch.
> 
> @Bryan, @Vishnu, plaese, can you review/ack also this patch?

This patch looks good to me.

Thank you, Arseniy for running the new test with VMCI.  I also ran some
of our internal tests successfully with this patch series.

> Thanks,
> Stefano
> 
>> 
>> Signed-off-by: Arseniy Krasnov 

Reviewed-by: Vishnu Dasa 

>> ---
>> net/vmw_vsock/vmci_transport_notify.c| 2 +-
>> net/vmw_vsock/vmci_transport_notify_qstate.c | 4 ++--
>> 2 files changed, 3 insertions(+), 3 deletions(-)
>> 
>> diff --git a/net/vmw_vsock/vmci_transport_notify.c 
>> b/net/vmw_vsock/vmci_transport_notify.c
>> index 852097e2b9e6..7c3a7db134b2 100644
>> --- a/net/vmw_vsock/vmci_transport_notify.c
>> +++ b/net/vmw_vsock/vmci_transport_notify.c
>> @@ -307,7 +307,7 @@ vmci_transport_handle_wrote(struct sock *sk,
>>  struct vsock_sock *vsk = vsock_sk(sk);
>>  PKT_FIELD(vsk, sent_waiting_read) = false;
>> #endif
>> -  sk->sk_data_ready(sk);
>> +  vsock_data_ready(sk);
>> }
>> 
>> static void vmci_transport_notify_pkt_socket_init(struct sock *sk)
>> diff --git a/net/vmw_vsock/vmci_transport_notify_qstate.c 
>> b/net/vmw_vsock/vmci_transport_notify_qstate.c
>> index 12f0cb8fe998..e96a88d850a8 100644
>> --- a/net/vmw_vsock/vmci_transport_notify_qstate.c
>> +++ b/net/vmw_vsock/vmci_transport_notify_qstate.c
>> @@ -84,7 +84,7 @@ vmci_transport_handle_wrote(struct sock *sk,
>>  bool bottom_half,
>>  struct sockaddr_vm *dst, struct sockaddr_vm *src)
>> {
>> -  sk->sk_data_ready(sk);
>> +  vsock_data_ready(sk);
>> }
>> 
>> static void vsock_block_update_write_window(struct sock *sk)
>> @@ -282,7 +282,7 @@ vmci_transport_notify_pkt_recv_post_dequeue(
>>  /* See the comment in
>>   * vmci_transport_notify_pkt_send_post_enqueue().
>>   */
>> -  sk->sk_data_ready(sk);
>> +  vsock_data_ready(sk);
>>  }
>> 
>>  return err;
>> --
>> 2.25.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: Potential deadlock in Linux VMCI driver

2022-08-16 Thread Vishnu Dasa via Virtualization


> On Aug 16, 2022, at 11:23 AM, Nadav Amit  wrote:
> 
> During my development, I enabled some Linux kernel checkers, specifically
> the “sleep in atomic” checker.
> 
> I ran into unrelated issue that appears to be a result of commit
> 463713eb6164b6 ("VMCI: dma dg: add support for DMA datagrams receive”).
> IIUC, vmci_read_data() calls wait_event(), which is not allowed while IRQs
> are disabled, which they are during IRQ handling.
> 
> I think "CONFIG_DEBUG_ATOMIC_SLEEP=y" is the one that triggers the warning
> below, which indicates a deadlock is possible.
> 
> The splat below (after decoding) was experienced on Linux 5.19. Let me know
> if you need me to open a bug in bugzilla or whether this issue is already
> known.

Nathan reported this a few days ago, but we haven't gotten
around to it yet.  Could you please file an internal bugzilla PR for
this?

Nathan,
Sorry we didn't respond to your email.  Jorgen is no longer with
VMware and is not working on VMCI/VSOCKETS.  We will take
a look at this.

Nadav,
Rajesh isn't with VMware now either, removing him from cc.

> [   22.629691] BUG: sleeping function called from invalid context at 
> drivers/misc/vmw_vmci/vmci_guest.c:145
> [   22.633894] in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 775, 
> name: cloud-init
> [   22.638232] preempt_count: 100, expected: 0
> [   22.641887] RCU nest depth: 0, expected: 0
> [   22.645461] 1 lock held by cloud-init/775:
> [   22.649013] #0: 88810e057200 (>i_mutex_dir_key#6){}-{3:3}, 
> at: iterate_dir (fs/readdir.c:46) 
> [   22.653012] Preemption disabled at:
> [   22.653017] __do_softirq (kernel/softirq.c:504 kernel/softirq.c:548) 
> [   22.660264] CPU: 3 PID: 775 Comm: cloud-init Not tainted 5.19.0+ #3
> [   22.664004] Hardware name: VMware, Inc. VMware20,1/440BX Desktop Reference 
> Platform, BIOS VMW201.00V.20253199.B64.2208081742 08/08/2022
> [   22.671600] Call Trace:
> [   22.675165]  
> [   22.678681] dump_stack_lvl (lib/dump_stack.c:107 (discriminator 4)) 
> [   22.682303] dump_stack (lib/dump_stack.c:114) 
> [   22.685883] __might_resched.cold (kernel/sched/core.c:9822) 
> [   22.689500] __might_sleep (kernel/sched/core.c:9751 (discriminator 14)) 
> [   22.692961] vmci_read_data (./include/linux/kernel.h:110 
> drivers/misc/vmw_vmci/vmci_guest.c:145) vmw_vmci
> [   22.696461] ? vmci_interrupt_bm (drivers/misc/vmw_vmci/vmci_guest.c:121) 
> vmw_vmci
> [   22.699920] ? __this_cpu_preempt_check (lib/smp_processor_id.c:67) 
> [   22.703305] ? wake_up_var (./include/linux/list.h:292 
> ./include/linux/wait.h:129 kernel/sched/wait_bit.c:125 
> kernel/sched/wait_bit.c:193) 
> [   22.706526] ? cpuusage_read (kernel/sched/wait_bit.c:192) 
> [   22.709682] ? mark_held_locks (kernel/locking/lockdep.c:4234) 
> [   22.712779] vmci_dispatch_dgs (drivers/misc/vmw_vmci/vmci_guest.c:332) 
> vmw_vmci
> [   22.715923] tasklet_action_common.constprop.0 (kernel/softirq.c:799) 
> [   22.719008] ? vmci_read_data (drivers/misc/vmw_vmci/vmci_guest.c:308) 
> vmw_vmci
> [   22.722018] tasklet_action (kernel/softirq.c:819) 
> [   22.724865] __do_softirq (kernel/softirq.c:571) 
> [   22.727650] __irq_exit_rcu (kernel/softirq.c:445 kernel/softirq.c:650) 
> [   22.730348] irq_exit_rcu (kernel/softirq.c:664) 
> [   22.732947] common_interrupt (arch/x86/kernel/irq.c:240 (discriminator 
> 14)) 
> [   22.735513]  
> [   22.737879]  
> [   22.740141] asm_common_interrupt (./arch/x86/include/asm/idtentry.h:640) 
> [   22.742498] RIP: 0010:stack_trace_consume_entry (kernel/stacktrace.c:83) 
> [ 22.744891] Code: be 80 01 00 00 48 c7 c7 40 82 cd 82 48 89 e5 e8 7d 38 53 
> 00 5d c3 cc cc cc cc cc cc cc cc cc cc cc 0f 1f 44 00 00 55 48 89 e5 <41> 55 
> 49 89 f5 41 54 53 48 89 fb 48 83 c7 10 e8 23 e0 36 00 48 8d
> All code
> 
>   0:  be 80 01 00 00  mov$0x180,%esi
>   5:  48 c7 c7 40 82 cd 82mov$0x82cd8240,%rdi
>   c:  48 89 e5mov%rsp,%rbp
>   f:  e8 7d 38 53 00  call   0x533891
>  14:  5d  pop%rbp
>  15:  c3  ret
>  16:  cc  int3   
>  17:  cc  int3   
>  18:  cc  int3   
>  19:  cc  int3   
>  1a:  cc  int3   
>  1b:  cc  int3   
>  1c:  cc  int3   
>  1d:  cc  int3   
>  1e:  cc  int3   
>  1f:  cc  int3   
>  20:  cc  int3   
>  21:  0f 1f 44 00 00  nopl   0x0(%rax,%rax,1)
>  26:  55  push   %rbp
>  27:  48 89 e5mov%rsp,%rbp
>  2a:* 41 55   push   %r13 <-- trapping instruction
>  2c:  49 89 f5mov%rsi,%r13
>  2f:  41 54   push   %r12
>  31:  53  push   %rbx
>  32:  48 89 fbmov%rdi,%rbx
>  35:  48 83 c7 10 add$0x10,%rdi
>  39:  e8 23 e0 36 00 

Re: [RFC PATCH v2 0/9] vsock: updates for SO_RCVLOWAT handling

2022-08-01 Thread Vishnu Dasa via Virtualization


> On Jul 27, 2022, at 5:37 AM, Stefano Garzarella  wrote:
> 
> Hi Arseniy,
> 
> On Mon, Jul 25, 2022 at 07:54:05AM +, Arseniy Krasnov wrote:
>> Hello,
>> 
>> This patchset includes some updates for SO_RCVLOWAT:
>> 
>> 1) af_vsock:
>>  During my experiments with zerocopy receive, i found, that in some
>>  cases, poll() implementation violates POSIX: when socket has non-
>>  default SO_RCVLOWAT(e.g. not 1), poll() will always set POLLIN and
>>  POLLRDNORM bits in 'revents' even number of bytes available to read
>>  on socket is smaller than SO_RCVLOWAT value. In this case,user sees
>>  POLLIN flag and then tries to read data(for example using  'read()'
>>  call), but read call will be blocked, because  SO_RCVLOWAT logic is
>>  supported in dequeue loop in af_vsock.c. But the same time,  POSIX
>>  requires that:
>> 
>>  "POLLIN Data other than high-priority data may be read without
>>  blocking.
>>   POLLRDNORM Normal data may be read without blocking."
>> 
>>  See 
>> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.open-std.org%2Fjtc1%2Fsc22%2Fopen%2Fn4217.pdfdata=05%7C01%7Cvdasa%40vmware.com%7C5ad2c6759fd8439e938708da6fccbee4%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C1%7C637945222450930014%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=60hG3DiYufOCv1DuufSdujiLEKDNou1Ztyah3GPbOLI%3Dreserved=0,
>>  page 293.
>> 
>>  So, we have, that poll() syscall returns POLLIN, but read call will
>>  be blocked.
>> 
>>  Also in man page socket(7) i found that:
>> 
>>  "Since Linux 2.6.28, select(2), poll(2), and epoll(7) indicate a
>>  socket as readable only if at least SO_RCVLOWAT bytes are available."
>> 
>>  I checked TCP callback for poll()(net/ipv4/tcp.c, tcp_poll()), it
>>  uses SO_RCVLOWAT value to set POLLIN bit, also i've tested TCP with
>>  this case for TCP socket, it works as POSIX required.
>> 
>>  I've added some fixes to af_vsock.c and virtio_transport_common.c,
>>  test is also implemented.
>> 
>> 2) virtio/vsock:
>>  It adds some optimization to wake ups, when new data arrived. Now,
>>  SO_RCVLOWAT is considered before wake up sleepers who wait new data.
>>  There is no sense, to kick waiter, when number of available bytes
>>  in socket's queue < SO_RCVLOWAT, because if we wake up reader in
>>  this case, it will wait for SO_RCVLOWAT data anyway during dequeue,
>>  or in poll() case, POLLIN/POLLRDNORM bits won't be set, so such
>>  exit from poll() will be "spurious". This logic is also used in TCP
>>  sockets.
> 
> Nice, it looks good!
> 
>> 
>> 3) vmci/vsock:
>>  Same as 2), but i'm not sure about this changes. Will be very good,
>>  to get comments from someone who knows this code.
> 
> I CCed VMCI maintainers to the patch and also to this cover, maybe
> better to keep them in the loop for next versions.
> 
> (Jorgen's and Rajesh's emails bounced back, so I'm CCing here only
> Bryan, Vishnu, and pv-driv...@vmware.com)

Hi Stefano,
Jorgen and Rajesh are no longer with VMware.  There's a patch in
flight to remove Rajesh from the MAINTAINERS file (Jorgen is already
removed).

Thanks,
Vishnu
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [RFC PATCH v2 0/9] vsock: updates for SO_RCVLOWAT handling

2022-08-01 Thread Vishnu Dasa via Virtualization



> On Jul 27, 2022, at 11:08 PM, Arseniy Krasnov  
> wrote:
> 
> On 27.07.2022 15:37, Stefano Garzarella wrote:
>> Hi Arseniy,
>> 
>> On Mon, Jul 25, 2022 at 07:54:05AM +, Arseniy Krasnov wrote:
>>> Hello,
>>> 
>>> This patchset includes some updates for SO_RCVLOWAT:
>>> 
>>> 1) af_vsock:
>>> During my experiments with zerocopy receive, i found, that in some
>>> cases, poll() implementation violates POSIX: when socket has non-
>>> default SO_RCVLOWAT(e.g. not 1), poll() will always set POLLIN and
>>> POLLRDNORM bits in 'revents' even number of bytes available to read
>>> on socket is smaller than SO_RCVLOWAT value. In this case,user sees
>>> POLLIN flag and then tries to read data(for example using 'read()'
>>> call), but read call will be blocked, because SO_RCVLOWAT logic is
>>> supported in dequeue loop in af_vsock.c. But the same time, POSIX
>>> requires that:
>>> 
>>> "POLLIN Data other than high-priority data may be read without
>>> blocking.
>>> POLLRDNORM Normal data may be read without blocking."
>>> 
>>> See 
>>> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.open-std.org%2Fjtc1%2Fsc22%2Fopen%2Fn4217.pdfdata=05%7C01%7Cvdasa%40vmware.com%7Cae83621d8709421de14b08da705faa9c%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C1%7C637945853473740235%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=NrbycCcVXV9Tz8NRDYBpnDx7KpFF6BZpSRbuhz1IfJ4%3Dreserved=0,
>>>  page 293.
>>> 
>>> So, we have, that poll() syscall returns POLLIN, but read call will
>>> be blocked.
>>> 
>>> Also in man page socket(7) i found that:
>>> 
>>> "Since Linux 2.6.28, select(2), poll(2), and epoll(7) indicate a
>>> socket as readable only if at least SO_RCVLOWAT bytes are available."
>>> 
>>> I checked TCP callback for poll()(net/ipv4/tcp.c, tcp_poll()), it
>>> uses SO_RCVLOWAT value to set POLLIN bit, also i've tested TCP with
>>> this case for TCP socket, it works as POSIX required.
>>> 
>>> I've added some fixes to af_vsock.c and virtio_transport_common.c,
>>> test is also implemented.
>>> 
>>> 2) virtio/vsock:
>>> It adds some optimization to wake ups, when new data arrived. Now,
>>> SO_RCVLOWAT is considered before wake up sleepers who wait new data.
>>> There is no sense, to kick waiter, when number of available bytes
>>> in socket's queue < SO_RCVLOWAT, because if we wake up reader in
>>> this case, it will wait for SO_RCVLOWAT data anyway during dequeue,
>>> or in poll() case, POLLIN/POLLRDNORM bits won't be set, so such
>>> exit from poll() will be "spurious". This logic is also used in TCP
>>> sockets.
>> 
>> Nice, it looks good!
> Thank You!
>> 
>>> 
>>> 3) vmci/vsock:
>>> Same as 2), but i'm not sure about this changes. Will be very good,
>>> to get comments from someone who knows this code.
>> 
>> I CCed VMCI maintainers to the patch and also to this cover, maybe better to 
>> keep them in the loop for next versions.
>> 
>> (Jorgen's and Rajesh's emails bounced back, so I'm CCing here only Bryan, 
>> Vishnu, and pv-driv...@vmware.com)
> Ok, i'll CC them in the next version
>> 
>>> 
>>> 4) Hyper-V:
>>> As Dexuan Cui mentioned, for Hyper-V transport it is difficult to
>>> support SO_RCVLOWAT, so he suggested to disable this feature for
>>> Hyper-V.
>> 
>> I left a couple of comments in some patches, but it seems to me to be in a 
>> good state :-)
>> 
>> I would just suggest a bit of a re-organization of the series (the patches 
>> are fine, just the order):
>> - introduce vsock_set_rcvlowat()
>> - disabling it for hv_sock
>> - use 'target' in virtio transports
>> - use 'target' in vmci transports
>> - use sock_rcvlowat in vsock_poll()
>> I think is better to pass sock_rcvlowat() as 'target' when the
>> transports are already able to use it
>> - add vsock_data_ready()
>> - use vsock_data_ready() in virtio transports
>> - use vsock_data_ready() in vmci transports
>> - tests
>> 
>> What do you think?
> No problem! I think i can wait for reply from VMWare guys before preparing v3

Looks fine to me, especially the VMCI parts.  Please send v3, and we can test it
from VMCI point of view as well.

Thanks,
Vishnu
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH] VMCI: Update maintainers for VMCI

2022-07-26 Thread Vishnu Dasa via Virtualization



> On Jul 26, 2022, at 9:18 AM, Joe Perches  wrote:
> 
> On Tue, 2022-07-26 at 15:55 +0000, Vishnu Dasa wrote:
>>> On Jul 26, 2022, at 8:10 AM, Greg KH >> On Mon, Jul 25, 2022 at 06:29:25PM +, Vishnu Dasa wrote:
>>>>> On Jul 25, 2022, at 11:05 AM, Greg KH  wrote:
>>>>> On Mon, Jul 25, 2022 at 09:32:46AM -0700, vd...@vmware.com wrote:
>>>>>> From: Vishnu Dasa 
>>>>>> Remove Rajesh as a maintainer for the VMCI driver.
>>>>> Why?
>>>> Rajesh is no longer with VMware and won't be working on VMCI.
>>> 
>>> But employment does not matter for maintainership and has nothing to do
>>> with it. Maintainership follows the person, not the company, you all
>>> know this.
>>> 
>>> So for obvious reasons, I can't take this type of change without
>>> Rajesh acking it.
>> 
>> I understand. After getting in touch with Rajesh, cc'ing him via his
>> personal email ID.
>> 
>> Rajesh, could you please provide your ack if you agree with this patch to
>> remove you as the maintainer for VMCI?
> 
> If being an employee of a company is a criteria for maintainership
> of this subsystem, likely the subsystem entry should be:
> 
> "S: Supported" not "S: Maintained"
> 
> MAINTAINERS:VMWARE VMCI DRIVER
> MAINTAINERS-M: Bryan Tan 
> MAINTAINERS-M: Rajesh Jalisatgi 
> MAINTAINERS-M: Vishnu Dasa 
> MAINTAINERS-R: VMware PV-Drivers Reviewers 
> MAINTAINERS-L: linux-ker...@vger.kernel.org
> MAINTAINERS-S: Maintained
> 
> Likely that's true for every VMware entry.
> 
> MAINTAINERS:VMWARE BALLOON DRIVER
> MAINTAINERS-M: Nadav Amit 
> MAINTAINERS-R: VMware PV-Drivers Reviewers 
> MAINTAINERS-L: linux-ker...@vger.kernel.org
> MAINTAINERS-S: Maintained
> MAINTAINERS-F: drivers/misc/vmw_balloon.c
> MAINTAINERS-
> MAINTAINERS:VMWARE PVRDMA DRIVER
> MAINTAINERS-M: Bryan Tan 
> MAINTAINERS-M: Vishnu Dasa 
> MAINTAINERS-R: VMware PV-Drivers Reviewers 
> MAINTAINERS-L: linux-r...@vger.kernel.org
> MAINTAINERS-S: Maintained
> MAINTAINERS-F: drivers/infiniband/hw/vmw_pvrdma/
> MAINTAINERS-
> MAINTAINERS-VMware PVSCSI driver
> MAINTAINERS-M: Vishal Bhakta 
> MAINTAINERS-R: VMware PV-Drivers Reviewers 
> MAINTAINERS-L: linux-s...@vger.kernel.org
> MAINTAINERS-S: Maintained
> MAINTAINERS-F: drivers/scsi/vmw_pvscsi.c
> MAINTAINERS-F: drivers/scsi/vmw_pvscsi.h
> MAINTAINERS-
> MAINTAINERS:VMWARE VMMOUSE SUBDRIVER
> MAINTAINERS-M: Zack Rusin 
> MAINTAINERS-R: VMware Graphics Reviewers 
> 
> MAINTAINERS-R: VMware PV-Drivers Reviewers 
> MAINTAINERS-L: linux-in...@vger.kernel.org
> MAINTAINERS-S: Maintained
> MAINTAINERS-F: drivers/input/mouse/vmmouse.c
> MAINTAINERS-F: drivers/input/mouse/vmmouse.h
> MAINTAINERS-
> MAINTAINERS:VMWARE VMXNET3 ETHERNET DRIVER
> MAINTAINERS-M: Ronak Doshi 
> MAINTAINERS-R: VMware PV-Drivers Reviewers 
> MAINTAINERS-L: net...@vger.kernel.org
> MAINTAINERS-S: Maintained
> MAINTAINERS-F: drivers/net/vmxnet3/
> MAINTAINERS-

Thanks for pointing that out.  Will discuss this with others and send a separate
patch out.

Thanks,
Vishnu
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH] VMCI: Update maintainers for VMCI

2022-07-26 Thread Vishnu Dasa via Virtualization


> On Jul 26, 2022, at 8:10 AM, Greg KH  wrote:
> 
> On Mon, Jul 25, 2022 at 06:29:25PM +0000, Vishnu Dasa wrote:
>> 
>>> On Jul 25, 2022, at 11:05 AM, Greg KH  wrote:
>>> 
>>> On Mon, Jul 25, 2022 at 09:32:46AM -0700, vd...@vmware.com wrote:
>>>> From: Vishnu Dasa 
>>>> 
>>>> Remove Rajesh as a maintainer for the VMCI driver.
>>> 
>>> Why?
>> 
>> Rajesh is no longer with VMware and won't be working on VMCI.
> 
> But employment does not matter for maintainership and has nothing to do
> with it.  Maintainership follows the person, not the company, you all
> know this.
> 
> So for obvious reasons, I can't take this type of change without Rajesh
> acking it.

I understand.  After getting in touch with Rajesh, cc'ing him via his personal
email ID.

Rajesh, could you please provide your ack if you agree with this patch to
remove you as the maintainer for VMCI?

Thanks,
Vishnu
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH] VMCI: Update maintainers for VMCI

2022-07-25 Thread Vishnu Dasa via Virtualization


> On Jul 25, 2022, at 11:05 AM, Greg KH  wrote:
> 
> On Mon, Jul 25, 2022 at 09:32:46AM -0700, vd...@vmware.com wrote:
>> From: Vishnu Dasa 
>> 
>> Remove Rajesh as a maintainer for the VMCI driver.
> 
> Why?

Rajesh is no longer with VMware and won't be working on VMCI.

> 
>> 
>> Acked-by: Bryan Tan 
>> Signed-off-by: Vishnu Dasa 
> 
> I need an ack from the person that is being removed here, for obvious
> reasons.  Any specific reason you didn't cc: them on this patch?

He is aware of this change, but we missed sending this patch before his
last day.  So, his email ID is inactive now.  Hence, it was omitted.

Please let me know if there's anything I can do.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH] VMCI: Add support for ARM64

2022-05-11 Thread Vishnu Dasa


> FWIW, it seems you're doing three things at once, better split this into
> a 3-patch series.

Thanks for the feedback.  I was debating between the two ways of doing
it and ultimately did it one way.  It is a bit late now to change it as it has
made its way to linux-next already.

Thanks,
Vishnu
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH] MAINTAINERS: Update for VMCI driver

2021-07-20 Thread Vishnu Dasa

On Jul 20, 2021, at 3:39 AM, Greg KH 
mailto:gre...@linuxfoundation.org>> wrote:

Also I would need an ack from Vishnu.

Acked-by: Vishnu Dasa mailto:d...@vmware.com>>

Thanks,
Vishnu

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH] VMCI: Fix integer overflow in VMCI handle arrays

2019-05-24 Thread Vishnu DASA via Virtualization
The VMCI handle array has an integer overflow in
vmci_handle_arr_append_entry when it tries to expand the array. This can be
triggered from a guest, since the doorbell link hypercall doesn't impose a
limit on the number of doorbell handles that a VM can create in the
hypervisor, and these handles are stored in a handle array.

In this change, we introduce a mandatory max capacity for handle
arrays/lists to avoid excessive memory usage.

Signed-off-by: Vishnu Dasa 
Reviewed-by: Adit Ranadive 
Reviewed-by: Jorgen Hansen 
---
 drivers/misc/vmw_vmci/vmci_context.c  | 80 +--
 drivers/misc/vmw_vmci/vmci_handle_array.c | 38 +++
 drivers/misc/vmw_vmci/vmci_handle_array.h | 29 +---
 include/linux/vmw_vmci_defs.h | 11 +++-
 4 files changed, 99 insertions(+), 59 deletions(-)

diff --git a/drivers/misc/vmw_vmci/vmci_context.c 
b/drivers/misc/vmw_vmci/vmci_context.c
index 21d0fa592145..bc089e634a75 100644
--- a/drivers/misc/vmw_vmci/vmci_context.c
+++ b/drivers/misc/vmw_vmci/vmci_context.c
@@ -29,6 +29,9 @@
 #include "vmci_driver.h"
 #include "vmci_event.h"
 
+/* Use a wide upper bound for the maximum contexts. */
+#define VMCI_MAX_CONTEXTS 2000
+
 /*
  * List of current VMCI contexts.  Contexts can be added by
  * vmci_ctx_create() and removed via vmci_ctx_destroy().
@@ -125,19 +128,22 @@ struct vmci_ctx *vmci_ctx_create(u32 cid, u32 priv_flags,
/* Initialize host-specific VMCI context. */
init_waitqueue_head(>host_context.wait_queue);
 
-   context->queue_pair_array = vmci_handle_arr_create(0);
+   context->queue_pair_array =
+   vmci_handle_arr_create(0, VMCI_MAX_GUEST_QP_COUNT);
if (!context->queue_pair_array) {
error = -ENOMEM;
goto err_free_ctx;
}
 
-   context->doorbell_array = vmci_handle_arr_create(0);
+   context->doorbell_array =
+   vmci_handle_arr_create(0, VMCI_MAX_GUEST_DOORBELL_COUNT);
if (!context->doorbell_array) {
error = -ENOMEM;
goto err_free_qp_array;
}
 
-   context->pending_doorbell_array = vmci_handle_arr_create(0);
+   context->pending_doorbell_array =
+   vmci_handle_arr_create(0, VMCI_MAX_GUEST_DOORBELL_COUNT);
if (!context->pending_doorbell_array) {
error = -ENOMEM;
goto err_free_db_array;
@@ -212,7 +218,7 @@ static int ctx_fire_notification(u32 context_id, u32 
priv_flags)
 * We create an array to hold the subscribers we find when
 * scanning through all contexts.
 */
-   subscriber_array = vmci_handle_arr_create(0);
+   subscriber_array = vmci_handle_arr_create(0, VMCI_MAX_CONTEXTS);
if (subscriber_array == NULL)
return VMCI_ERROR_NO_MEM;
 
@@ -631,20 +637,26 @@ int vmci_ctx_add_notification(u32 context_id, u32 
remote_cid)
 
spin_lock(>lock);
 
-   list_for_each_entry(n, >notifier_list, node) {
-   if (vmci_handle_is_equal(n->handle, notifier->handle)) {
-   exists = true;
-   break;
+   if (context->n_notifiers < VMCI_MAX_CONTEXTS) {
+   list_for_each_entry(n, >notifier_list, node) {
+   if (vmci_handle_is_equal(n->handle, notifier->handle)) {
+   exists = true;
+   break;
+   }
}
-   }
 
-   if (exists) {
-   kfree(notifier);
-   result = VMCI_ERROR_ALREADY_EXISTS;
+   if (exists) {
+   kfree(notifier);
+   result = VMCI_ERROR_ALREADY_EXISTS;
+   } else {
+   list_add_tail_rcu(>node,
+ >notifier_list);
+   context->n_notifiers++;
+   result = VMCI_SUCCESS;
+   }
} else {
-   list_add_tail_rcu(>node, >notifier_list);
-   context->n_notifiers++;
-   result = VMCI_SUCCESS;
+   kfree(notifier);
+   result = VMCI_ERROR_NO_MEM;
}
 
spin_unlock(>lock);
@@ -729,8 +741,7 @@ static int vmci_ctx_get_chkpt_doorbells(struct vmci_ctx 
*context,
u32 *buf_size, void **pbuf)
 {
struct dbell_cpt_state *dbells;
-   size_t n_doorbells;
-   int i;
+   u32 i, n_doorbells;
 
n_doorbells = vmci_handle_arr_get_size(context->doorbell_array);
if (n_doorbells > 0) {
@@ -868,7 +879,8 @@ int vmci_ctx_rcv_notifications_get(u32 context_id,
spin_lock(>lock);
 
*db_handle_array = context->pending_doorbell_array;
-   context->pending_doorbell_array = vmci_handle_arr_create(0);
+   context->pending_doorbell_array =
+