From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933014AbbJHNxE (ORCPT ); Thu, 8 Oct 2015 09:53:04 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48660 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932876AbbJHNxC (ORCPT ); Thu, 8 Oct 2015 09:53:02 -0400 From: Vitaly Kuznetsov To: Olaf Hering Cc: "K. Y. Srinivasan" , gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org, devel@linuxdriverproject.org, apw@canonical.com, jasowang@redhat.com Subject: Re: [PATCH 02/10] Drivers: hv: utils: run polling callback always in interrupt context References: <1444269571-25217-1-git-send-email-kys@microsoft.com> <1444269709-25270-1-git-send-email-kys@microsoft.com> <1444269709-25270-2-git-send-email-kys@microsoft.com> <87r3l5ebmj.fsf@vitty.brq.redhat.com> <20151008133037.GA31748@aepfle.de> Date: Thu, 08 Oct 2015 15:52:57 +0200 In-Reply-To: <20151008133037.GA31748@aepfle.de> (Olaf Hering's message of "Thu, 8 Oct 2015 15:30:37 +0200") Message-ID: <87mvvteabq.fsf@vitty.brq.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Olaf Hering writes: > On Thu, Oct 08, Vitaly Kuznetsov wrote: > >> > @@ -295,9 +288,6 @@ static int fcopy_on_msg(void *msg, int len) >> > if (fcopy_transaction.state == HVUTIL_DEVICE_INIT) >> > return fcopy_handle_handshake(*val); >> > >> > - if (fcopy_transaction.state != HVUTIL_USERSPACE_REQ) >> > - return -EINVAL; >> > - >> >> This particular change seems unrelated and I'm unsure it's safe to >> remove this check. It is meant to protect against daemon screwing the >> protocol and writing to the device when it wasn't requested for an >> action. It is correct to propagate -EINVAL in this case. Or am I missing >> something and the check is redundant now? > > What can happen if there is an odd write request? I think we don't want to propagate misbehaving daemon's data to the host -- let's cut it here. E.g. imagine there is no communication going on and daemon starts writing something to the device. In case we remove the check we'll be doing fcopy_respond_to_host() for each daemon's write flooding the host. > If there is a timeout > scheduled some return value will be sent to the host. Then the state is > set to RESET and eventually vmbus_recvpacket will receive something. > That something will be processed and passed to the daemon. > > If there was no timeout scheduled the write will just return. yes, but after doing fcopy_respond_to_host(). I'd suggest we leave the check in place, better safe than sorry. -- Vitaly