Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200724164606.43699-1-lkmlabelt@gmail.com> <20200724101047.34de7e49@hermes.lan>
In-Reply-To: <20200724101047.34de7e49@hermes.lan>
From:   Andres Beltran <lkmlabelt@gmail.com>
Date:   Fri, 24 Jul 2020 19:04:27 -0400
Message-ID: <CAGpZZ6ugh-SprR=oMkktEVuEJvNrK06TLqgskey0JkCdZCmvMA@mail.gmail.com>
Subject: Re: [PATCH] Drivers: hv: vmbus: Fix variable assignments in hv_ringbuffer_read()
To:     Stephen Hemminger <stephen@networkplumber.org>
Cc:     KY Srinivasan <kys@microsoft.com>,
        Haiyang Zhang <haiyangz@microsoft.com>,
        Stephen Hemminger <sthemmin@microsoft.com>,
        Wei Liu <wei.liu@kernel.org>, linux-hyperv@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Michael Kelley <mikelley@microsoft.com>,
        Andrea Parri <parri.andrea@gmail.com>,
        Saruhan Karademir <skarade@microsoft.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Jul 24, 2020 at 1:10 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
> What is the rationale for this change, it may break other code.
>
> A common API model in Windows world where this originated
> is to have a call where caller first
> makes request and then if the requested buffer is not big enough the
> caller look at the actual length and allocate a bigger buffer.
>
> Did you audit all the users of this API to make sure they aren't doing that.
>

The rationale for the change was to solve instances like the one
@Haiyang Zhang pointed out, especially in hv_utils, which needs
additional hardening. Unfortunately, there is an instance in
hv_pci_onchannelcallback() that does what you just described. Thus,
the fix will have to be made to all the callers of vmbus_recvpacket()
and vmbus_recvpacket_raw() to make sure they check the return value,
which most callers are not doing now. Thanks for pointing out this
behavior. I was not aware that the length can be checked by callers to
allocate a bigger buffer.