From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933809AbbFJRKX (ORCPT ); Wed, 10 Jun 2015 13:10:23 -0400 Received: from mga11.intel.com ([192.55.52.93]:13227 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754378AbbFJRKQ (ORCPT ); Wed, 10 Jun 2015 13:10:16 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,588,1427785200"; d="scan'208";a="724749910" From: "Grumbach, Emmanuel" To: "xerofoify@gmail.com" CC: "linux-kernel@vger.kernel.org" , "Spinadel, David" , "ilw@linux.intel.com" , "Coelho, Luciano" , "eliad@wizery.com" , "Berg, Johannes" , "kvalo@codeaurora.org" , "netdev@vger.kernel.org" , "linux-wireless@vger.kernel.org" Subject: Re: [PATCH] iwlwifi:dvm:Return false if resume command data is not same size as received packet for the function iwl_resume_status_fn Thread-Topic: [PATCH] iwlwifi:dvm:Return false if resume command data is not same size as received packet for the function iwl_resume_status_fn Thread-Index: AQHQo5tGhwssydsYo0O43yhmx8as5Z2lwa6AgAACBwCAAANlgA== Date: Wed, 10 Jun 2015 17:10:09 +0000 Message-ID: <1433956209.20602.5.camel@intel.com> References: <1433954031-7176-1-git-send-email-xerofoify@gmail.com> <1433955045.20602.2.camel@intel.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.254.148.89] Content-Type: text/plain; charset="utf-8" Content-ID: MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by nfs id t5AHASjD006024 On Wed, 2015-06-10 at 12:58 -0400, Nicholas Krause wrote: > > On June 10, 2015 12:50:45 PM EDT, "Grumbach, Emmanuel" wrote: > >On Wed, 2015-06-10 at 12:33 -0400, Nicholas Krause wrote: > >> This makes the function iwl_resume_status_fn return false now if > >> the received packet of type iwl_rx_packet is not the same size > >> as the structure pointer, iwl_resume_data's cmd element in order > >> to signal callers about this error and allow them to handle it > >> occurrently. > >> > > > >Hm... Did you actually hit this if? > >I am not sure I really want to wait here (which is what will happen if > >you return false) when we get an unexpected length? I do not expect > >anything besides the response I am waiting for since the firmware is > >handling the GET_STATUS *only* - it just came back from WoWLAN. Bottom > >line, this is really an error path and I prefer to exit and not wait > >for > >the timeout in that case. > >But I might be missing something? > > > Why not wait for the time out? Seems there is no reason not to and in that case > if the firmware handles this I doubt it will. This goes back to my original question: did you really hit this path? Does the patch solves a real bug you faced? > why is the if statement still here. To get a debug print? :) > Nick > >> Signed-off-by: Nicholas Krause > >> --- > >> drivers/net/wireless/iwlwifi/dvm/mac80211.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/net/wireless/iwlwifi/dvm/mac80211.c > >b/drivers/net/wireless/iwlwifi/dvm/mac80211.c > >> index 5abd62e..21e808c 100644 > >> --- a/drivers/net/wireless/iwlwifi/dvm/mac80211.c > >> +++ b/drivers/net/wireless/iwlwifi/dvm/mac80211.c > >> @@ -409,7 +409,7 @@ static bool iwl_resume_status_fn(struct > >iwl_notif_wait_data *notif_wait, > >> > >> if (iwl_rx_packet_payload_len(pkt) != sizeof(*resume_data->cmd)) { > >> IWL_ERR(priv, "rx wrong size data\n"); > >> - return true; > >> + return false; > >> } > >> memcpy(resume_data->cmd, pkt->data, sizeof(*resume_data->cmd)); > >> resume_data->valid = true; > {.n++%ݶw{.n+{G{ayʇڙ,jfhz_(階ݢj"mG?&~iOzv^m ?I