Re: [PATCH] iwlwifi:dvm:Return false if resume command data is not same size as received packet for the function iwl_resume_status_fn

All of lore.kernel.org
 help / color / mirror / Atom feed

From: "Grumbach, Emmanuel" <emmanuel.grumbach@intel.com>
To: "xerofoify@gmail.com" <xerofoify@gmail.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Spinadel, David" <david.spinadel@intel.com>,
	"Coelho, Luciano" <luciano.coelho@intel.com>,
	"ilw@linux.intel.com" <ilw@linux.intel.com>,
	"eliad@wizery.com" <eliad@wizery.com>,
	"Berg, Johannes" <johannes.berg@intel.com>,
	"kvalo@codeaurora.org" <kvalo@codeaurora.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-wireless@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
Date: Wed, 10 Jun 2015 16:50:45 +0000	[thread overview]
Message-ID: <1433955045.20602.2.camel@intel.com> (raw)
In-Reply-To: <1433954031-7176-1-git-send-email-xerofoify@gmail.com>

T24gV2VkLCAyMDE1LTA2LTEwIGF0IDEyOjMzIC0wNDAwLCBOaWNob2xhcyBLcmF1c2Ugd3JvdGU6
DQo+IFRoaXMgbWFrZXMgdGhlIGZ1bmN0aW9uIGl3bF9yZXN1bWVfc3RhdHVzX2ZuIHJldHVybiBm
YWxzZSBub3cgaWYNCj4gdGhlIHJlY2VpdmVkIHBhY2tldCBvZiB0eXBlIGl3bF9yeF9wYWNrZXQg
aXMgbm90IHRoZSBzYW1lIHNpemUNCj4gYXMgdGhlIHN0cnVjdHVyZSBwb2ludGVyLCBpd2xfcmVz
dW1lX2RhdGEncyBjbWQgZWxlbWVudCBpbiBvcmRlcg0KPiB0byBzaWduYWwgY2FsbGVycyBhYm91
dCB0aGlzIGVycm9yIGFuZCBhbGxvdyB0aGVtIHRvIGhhbmRsZSBpdA0KPiBvY2N1cnJlbnRseS4N
Cj4gDQoNCkhtLi4uIERpZCB5b3UgYWN0dWFsbHkgaGl0IHRoaXMgaWY/DQpJIGFtIG5vdCBzdXJl
IEkgcmVhbGx5IHdhbnQgdG8gd2FpdCBoZXJlICh3aGljaCBpcyB3aGF0IHdpbGwgaGFwcGVuIGlm
DQp5b3UgcmV0dXJuIGZhbHNlKSB3aGVuIHdlIGdldCBhbiB1bmV4cGVjdGVkIGxlbmd0aD8gSSBk
byBub3QgZXhwZWN0DQphbnl0aGluZyBiZXNpZGVzIHRoZSByZXNwb25zZSBJIGFtIHdhaXRpbmcg
Zm9yIHNpbmNlIHRoZSBmaXJtd2FyZSBpcw0KaGFuZGxpbmcgdGhlIEdFVF9TVEFUVVMgKm9ubHkq
IC0gaXQganVzdCBjYW1lIGJhY2sgZnJvbSBXb1dMQU4uIEJvdHRvbQ0KbGluZSwgdGhpcyBpcyBy
ZWFsbHkgYW4gZXJyb3IgcGF0aCBhbmQgSSBwcmVmZXIgdG8gZXhpdCBhbmQgbm90IHdhaXQgZm9y
DQp0aGUgdGltZW91dCBpbiB0aGF0IGNhc2UuDQpCdXQgSSBtaWdodCBiZSBtaXNzaW5nIHNvbWV0
aGluZz8NCg0KPiBTaWduZWQtb2ZmLWJ5OiBOaWNob2xhcyBLcmF1c2UgPHhlcm9mb2lmeUBnbWFp
bC5jb20+DQo+IC0tLQ0KPiAgZHJpdmVycy9uZXQvd2lyZWxlc3MvaXdsd2lmaS9kdm0vbWFjODAy
MTEuYyB8IDIgKy0NCj4gIDEgZmlsZSBjaGFuZ2VkLCAxIGluc2VydGlvbigrKSwgMSBkZWxldGlv
bigtKQ0KPiANCj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvbmV0L3dpcmVsZXNzL2l3bHdpZmkvZHZt
L21hYzgwMjExLmMgYi9kcml2ZXJzL25ldC93aXJlbGVzcy9pd2x3aWZpL2R2bS9tYWM4MDIxMS5j
DQo+IGluZGV4IDVhYmQ2MmUuLjIxZTgwOGMgMTAwNjQ0DQo+IC0tLSBhL2RyaXZlcnMvbmV0L3dp
cmVsZXNzL2l3bHdpZmkvZHZtL21hYzgwMjExLmMNCj4gKysrIGIvZHJpdmVycy9uZXQvd2lyZWxl
c3MvaXdsd2lmaS9kdm0vbWFjODAyMTEuYw0KPiBAQCAtNDA5LDcgKzQwOSw3IEBAIHN0YXRpYyBi
b29sIGl3bF9yZXN1bWVfc3RhdHVzX2ZuKHN0cnVjdCBpd2xfbm90aWZfd2FpdF9kYXRhICpub3Rp
Zl93YWl0LA0KPiAgDQo+ICAJaWYgKGl3bF9yeF9wYWNrZXRfcGF5bG9hZF9sZW4ocGt0KSAhPSBz
aXplb2YoKnJlc3VtZV9kYXRhLT5jbWQpKSB7DQo+ICAJCUlXTF9FUlIocHJpdiwgInJ4IHdyb25n
IHNpemUgZGF0YVxuIik7DQo+IC0JCXJldHVybiB0cnVlOw0KPiArCQlyZXR1cm4gZmFsc2U7DQo+
ICAJfQ0KPiAgCW1lbWNweShyZXN1bWVfZGF0YS0+Y21kLCBwa3QtPmRhdGEsIHNpemVvZigqcmVz
dW1lX2RhdGEtPmNtZCkpOw0KPiAgCXJlc3VtZV9kYXRhLT52YWxpZCA9IHRydWU7DQoNCg==

WARNING: multiple messages have this Message-ID (diff)

From: "Grumbach, Emmanuel" <emmanuel.grumbach@intel.com>
To: "xerofoify@gmail.com" <xerofoify@gmail.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Spinadel, David" <david.spinadel@intel.com>,
	"Coelho, Luciano" <luciano.coelho@intel.com>,
	"ilw@linux.intel.com" <ilw@linux.intel.com>,
	"eliad@wizery.com" <eliad@wizery.com>,
	"Berg, Johannes" <johannes.berg@intel.com>,
	"kvalo@codeaurora.org" <kvalo@codeaurora.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-wireless@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
Date: Wed, 10 Jun 2015 16:50:45 +0000	[thread overview]
Message-ID: <1433955045.20602.2.camel@intel.com> (raw)
In-Reply-To: <1433954031-7176-1-git-send-email-xerofoify@gmail.com>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1715 bytes --]

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?

> Signed-off-by: Nicholas Krause <xerofoify@gmail.com>
> ---
>  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Ç+‰·Ÿ®‰†+%ŠËÿ±éÝ¶\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dÊ‡Ú™ë,j\a¢f£¢·hšïêÿ‘êçz_è®\x03(éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨èÚ&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

WARNING: multiple messages have this Message-ID (diff)

From: "Grumbach, Emmanuel" <emmanuel.grumbach-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
To: "xerofoify-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org"
	<xerofoify-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"Spinadel,
	David" <david.spinadel-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
	"Coelho,
	Luciano" <luciano.coelho-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
	"ilw-VuQAYsv1563Yd54FQh9/CA@public.gmane.org"
	<ilw-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>,
	"eliad-Ix1uc/W3ht7QT0dZR+AlfA@public.gmane.org"
	<eliad-Ix1uc/W3ht7QT0dZR+AlfA@public.gmane.org>,
	"Berg,
	Johannes" <johannes.berg-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
	"kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org"
	<kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>,
	"netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.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
Date: Wed, 10 Jun 2015 16:50:45 +0000	[thread overview]
Message-ID: <1433955045.20602.2.camel@intel.com> (raw)
In-Reply-To: <1433954031-7176-1-git-send-email-xerofoify-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

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?

> Signed-off-by: Nicholas Krause <xerofoify@gmail.com>
> ---
>  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;

next      parent reply	other threads:[~2015-06-10 16:50 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1433954031-7176-1-git-send-email-xerofoify@gmail.com>
2015-06-10 16:50 ` Grumbach, Emmanuel [this message]
2015-06-10 16:50   ` [PATCH] iwlwifi:dvm:Return false if resume command data is not same size as received packet for the function iwl_resume_status_fn Grumbach, Emmanuel
2015-06-10 16:50   ` Grumbach, Emmanuel
     [not found]   ` <D665C857-C846-422A-A3D9-A9314B000CD6@gmail.com>
2015-06-10 17:10     ` Grumbach, Emmanuel
2015-06-10 17:10       ` Grumbach, Emmanuel
2015-06-10 17:10       ` Grumbach, Emmanuel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1433955045.20602.2.camel@intel.com \
    --to=emmanuel.grumbach@intel.com \
    --cc=david.spinadel@intel.com \
    --cc=eliad@wizery.com \
    --cc=ilw@linux.intel.com \
    --cc=johannes.berg@intel.com \
    --cc=kvalo@codeaurora.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=luciano.coelho@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=xerofoify@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Be sure your reply has a Subject: header at the top and a blank line before the message body.

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.