From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id E9573C83F27 for ; Wed, 16 Jul 2025 14:45:37 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id B4C7010E7BB; Wed, 16 Jul 2025 14:45:37 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (1024-bit key; unprotected) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.b="Vtgwqkmu"; dkim-atps=neutral Received: from sea.source.kernel.org (sea.source.kernel.org [172.234.252.31]) by gabe.freedesktop.org (Postfix) with ESMTPS id 4148810E7BF; Wed, 16 Jul 2025 14:45:35 +0000 (UTC) Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sea.source.kernel.org (Postfix) with ESMTP id AF9F744AEA; Wed, 16 Jul 2025 14:45:35 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 41021C4CEE7; Wed, 16 Jul 2025 14:45:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1752677135; bh=nHbkOPQThJAUcbsBsK/Y+mJAiUxpmcKEzvzLu/ss+WE=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=VtgwqkmuF7QRE0w2Tbk4VxeG8fUwHFknE2cGEUSTjp7oKwV+MYkABHgHkUTW9eIO2 Pr14N/jpdoXC/SXZN0xr89HZfiGZYiQiGWJKZ2RA9iaHSYo64rbdAdXci/PMNu3KBi uplbb2kA8XaR6Gk7qinoFa02kVtDhVi7aZSca+AA= Date: Wed, 16 Jul 2025 16:45:33 +0200 From: Greg KH To: "Usyskin, Alexander" Cc: "Vivi, Rodrigo" , "intel-xe@lists.freedesktop.org" , "dri-devel@lists.freedesktop.org" , "linux-kernel@vger.kernel.org" , "Ceraolo Spurio, Daniele" , "Gupta, Anshuman" , "Nilawar, Badal" Subject: Re: [PATCH 2/9] mei: late_bind: add late binding component driver Message-ID: <2025071619-sterile-skiing-e64b@gregkh> References: <20250710150831.3018674-11-rodrigo.vivi@intel.com> <20250710150831.3018674-13-rodrigo.vivi@intel.com> <2025071611-decode-hastiness-df63@gregkh> <2025071603-guide-definite-70e3@gregkh> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-BeenThere: intel-xe@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel Xe graphics driver List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-xe-bounces@lists.freedesktop.org Sender: "Intel-xe" On Wed, Jul 16, 2025 at 02:26:26PM +0000, Usyskin, Alexander wrote: > > > > > + if (bytes < sizeof(rsp)) { > > > > > + dev_err(dev, "bad response from the firmware: size %zd < > > > > %zu\n", > > > > > + bytes, sizeof(rsp)); > > > > > + ret = -EPROTO; > > > > > + goto end; > > > > > + } > > > > > > > > Why not check this above when you check against the size of the header? > > > > You only need one size check, not 2. > > > Firmware may return only header with result field set without the data. > > > > Then the firmware is broken :) > > > > > We are parsing the header first and then starting to parse data. > > > If we check for whole message size at the beginning we'll miss the result > > data. > > > > You mean you will make it harder to debug the firmware, as you will not > > be printing out the header information? Or something else? The > > bytes variable HAS to match the full structure size, not just the header > > size, according to this code. So just test for that and be done with > > it! > > The CSME firmware returns only command header if, like, command is not recognised. > This may happen because of firmware bug or for firmware is configured/compiled > that way. > This seems reasonable for the complex protocols where firmware may not be > aware of this particular command at all and have no idea what the data size it > should send in reply. > Printing result from the header will allow us to understand either this is the firmware > problem or driver sent something wrong. Then make it obvious in your checking and in your error messages as to what you are doing here. Checking the size of the buffer in two different places, with different values is very odd, and deserves a lot of explaination. thanks, greg k-h