Commit 7ec7c4a9a686c608315739ab6a2b0527a240883c (mac80211: port CCMP to
cryptoapi's CCM driver) introduced a regression when decrypting empty
packets (data_len == 0). This will lead to backtraces like:
(scatterwalk_start) from [<c01312f4>] (scatterwalk_map_and_copy+0x2c/0xa8)
(scatterwalk_map_and_copy) from [<c013a5a0>] (crypto_ccm_decrypt+0x7c/0x25c)
(crypto_ccm_decrypt) from [<c032886c>] (ieee80211_aes_ccm_decrypt+0x160/0x170)
(ieee80211_aes_ccm_decrypt) from [<c031c628>] (ieee80211_crypto_ccmp_decrypt+0x1ac/0x238)
(ieee80211_crypto_ccmp_decrypt) from [<c032ef28>] (ieee80211_rx_handlers+0x870/0x1d24)
(ieee80211_rx_handlers) from [<c0330c7c>] (ieee80211_prepare_and_rx_handle+0x8a0/0x91c)
(ieee80211_prepare_and_rx_handle) from [<c0331260>] (ieee80211_rx+0x568/0x730)
(ieee80211_rx) from [<c01d3054>] (__carl9170_rx+0x94c/0xa20)
(__carl9170_rx) from [<c01d3324>] (carl9170_rx_stream+0x1fc/0x320)
(carl9170_rx_stream) from [<c01cbccc>] (carl9170_usb_tasklet+0x80/0xc8)
(carl9170_usb_tasklet) from [<c00199dc>] (tasklet_hi_action+0x88/0xcc)
(tasklet_hi_action) from [<c00193c8>] (__do_softirq+0xcc/0x200)
(__do_softirq) from [<c0019734>] (irq_exit+0x80/0xe0)
(irq_exit) from [<c0009c10>] (handle_IRQ+0x64/0x80)
(handle_IRQ) from [<c000c3a0>] (__irq_svc+0x40/0x4c)
(__irq_svc) from [<c0009d44>] (arch_cpu_idle+0x2c/0x34)
Such packets can appear for example when using the carl9170 wireless driver
because hardware sometimes generates garbage when the internal FIFO overruns.
This patch adds an additional length check.
Signed-off-by: Ronald Wahl <[email protected]>
---
net/mac80211/aes_ccm.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/mac80211/aes_ccm.c b/net/mac80211/aes_ccm.c
index ec24378..0cd9523 100644
--- a/net/mac80211/aes_ccm.c
+++ b/net/mac80211/aes_ccm.c
@@ -53,6 +53,9 @@ int ieee80211_aes_ccm_decrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad,
__aligned(__alignof__(struct aead_request));
struct aead_request *aead_req = (void *) aead_req_data;
+ if (data_len == 0)
+ return EINVAL;
+
memset(aead_req, 0, sizeof(aead_req_data));
sg_init_one(&pt, data, data_len);
--
1.9.3
On 06.11.2014 11:02, Johannes Berg wrote:
> On Thu, 2014-11-06 at 10:55 +0100, Ronald Wahl wrote:
>> Commit 7ec7c4a9a686c608315739ab6a2b0527a240883c (mac80211: port CCMP to
>> cryptoapi's CCM driver) introduced a regression when decrypting empty
>> packets (data_len == 0). This will lead to backtraces like:
>
> Not good.
>
>> This patch adds an additional length check.
>>
>> Signed-off-by: Ronald Wahl <[email protected]>
>
> Please add
>
> Cc: [email protected]
> Fixes: 7ec7c4a9a686 ("mac80211: port CCMP to cryptoapi's CCM driver")
>
>> + if (data_len == 0)
>> + return EINVAL;
>
> error codes should be negative.
Argh. New patch on the way...
thx,
ron
--
Ronald Wahl - [email protected] - Phone +49 375271349-0 Fax -99
Raritan Deutschland GmbH, Kornmarkt 7, 08056 Zwickau, Germany
USt-IdNr. DE813094160, Steuer-Nr. 227/117/01749
Amtsgericht Chemnitz HRB 23605
Geschäftsführung: Stuart Hopper, Ralf Ploenes
On 06.11.2014 13:39, Johannes Berg wrote:
> On Thu, 2014-11-06 at 13:34 +0100, Arend van Spriel wrote:
>> On 06-11-14 11:02, Johannes Berg wrote:
>>> Oh, regarding Cc stable - don't actually send the mail there if you can
>>> help it.
>>
>> What do you mean? Are you saying: It must be tagged, but 'git
>> send-email' should not actually add the Cc field in the mail header.
>> What is the story here? Guess I missed a memo ;-)
>
> As far as I remember Greg just ignores the patches sent to the list and
> picks them up when they hit Linus's tree instead (i.e. follows the
> commit logs with like 'git log --grep "Cc: stable"') so there's no value
> in actually sending out email, it's just noise.
>
> So far it has worked for me, so I think it must be right :)
But there are LTS kernels not maintained by Greg like 3.12 and 3.16. How
about these? If sending patches to stable@kernel org is optional then
it's better to kill that list or silently drop emails send to this list
so no errors are returned for this address.
- ron
--
Ronald Wahl - [email protected] - Phone +49 375271349-0 Fax -99
Raritan Deutschland GmbH, Kornmarkt 7, 08056 Zwickau, Germany
USt-IdNr. DE813094160, Steuer-Nr. 227/117/01749
Amtsgericht Chemnitz HRB 23605
Geschäftsführung: Stuart Hopper, Ralf Ploenes
On 06-11-14 11:02, Johannes Berg wrote:
> Oh, regarding Cc stable - don't actually send the mail there if you can
> help it.
What do you mean? Are you saying: It must be tagged, but 'git
send-email' should not actually add the Cc field in the mail header.
What is the story here? Guess I missed a memo ;-)
> johannes
Regards,
Arend
On Thu, 2014-11-06 at 10:55 +0100, Ronald Wahl wrote:
> Commit 7ec7c4a9a686c608315739ab6a2b0527a240883c (mac80211: port CCMP to
> cryptoapi's CCM driver) introduced a regression when decrypting empty
> packets (data_len == 0). This will lead to backtraces like:
Not good.
> This patch adds an additional length check.
>
> Signed-off-by: Ronald Wahl <[email protected]>
Please add
Cc: [email protected]
Fixes: 7ec7c4a9a686 ("mac80211: port CCMP to cryptoapi's CCM driver")
> + if (data_len == 0)
> + return EINVAL;
error codes should be negative. Or we could just return 0 to pretend it
was decrypted, after all, there was nothing to do? The packet is useless
though, so might as well drop it (and we would later anyway)
Oh, regarding Cc stable - don't actually send the mail there if you can
help it.
johannes
On 6 November 2014 10:55, Ronald Wahl <[email protected]> wrote:
> Commit 7ec7c4a9a686c608315739ab6a2b0527a240883c (mac80211: port CCMP to
> cryptoapi's CCM driver) introduced a regression when decrypting empty
> packets (data_len == 0). This will lead to backtraces like:
>
> (scatterwalk_start) from [<c01312f4>] (scatterwalk_map_and_copy+0x2c/0xa8)
> (scatterwalk_map_and_copy) from [<c013a5a0>] (crypto_ccm_decrypt+0x7c/0x25c)
> (crypto_ccm_decrypt) from [<c032886c>] (ieee80211_aes_ccm_decrypt+0x160/0x170)
> (ieee80211_aes_ccm_decrypt) from [<c031c628>] (ieee80211_crypto_ccmp_decrypt+0x1ac/0x238)
> (ieee80211_crypto_ccmp_decrypt) from [<c032ef28>] (ieee80211_rx_handlers+0x870/0x1d24)
> (ieee80211_rx_handlers) from [<c0330c7c>] (ieee80211_prepare_and_rx_handle+0x8a0/0x91c)
> (ieee80211_prepare_and_rx_handle) from [<c0331260>] (ieee80211_rx+0x568/0x730)
> (ieee80211_rx) from [<c01d3054>] (__carl9170_rx+0x94c/0xa20)
> (__carl9170_rx) from [<c01d3324>] (carl9170_rx_stream+0x1fc/0x320)
> (carl9170_rx_stream) from [<c01cbccc>] (carl9170_usb_tasklet+0x80/0xc8)
> (carl9170_usb_tasklet) from [<c00199dc>] (tasklet_hi_action+0x88/0xcc)
> (tasklet_hi_action) from [<c00193c8>] (__do_softirq+0xcc/0x200)
> (__do_softirq) from [<c0019734>] (irq_exit+0x80/0xe0)
> (irq_exit) from [<c0009c10>] (handle_IRQ+0x64/0x80)
> (handle_IRQ) from [<c000c3a0>] (__irq_svc+0x40/0x4c)
> (__irq_svc) from [<c0009d44>] (arch_cpu_idle+0x2c/0x34)
>
> Such packets can appear for example when using the carl9170 wireless driver
> because hardware sometimes generates garbage when the internal FIFO overruns.
>
> This patch adds an additional length check.
>
> Signed-off-by: Ronald Wahl <[email protected]>
> ---
> net/mac80211/aes_ccm.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/net/mac80211/aes_ccm.c b/net/mac80211/aes_ccm.c
> index ec24378..0cd9523 100644
> --- a/net/mac80211/aes_ccm.c
> +++ b/net/mac80211/aes_ccm.c
> @@ -53,6 +53,9 @@ int ieee80211_aes_ccm_decrypt(struct crypto_aead *tfm, u8 *b_0, u8 *aad,
> __aligned(__alignof__(struct aead_request));
> struct aead_request *aead_req = (void *) aead_req_data;
>
> + if (data_len == 0)
> + return EINVAL;
> +
Did you mean -EINVAL?
--
Ard.
> memset(aead_req, 0, sizeof(aead_req_data));
>
> sg_init_one(&pt, data, data_len);
> --
> 1.9.3
>
On 06.11.2014 14:14, Johannes Berg wrote:
> On Thu, 2014-11-06 at 14:07 +0100, Ronald Wahl wrote:
>
>> But there are LTS kernels not maintained by Greg like 3.12 and 3.16. How
>> about these? If sending patches to stable@kernel org is optional then
>> it's better to kill that list or silently drop emails send to this list
>> so no errors are returned for this address.
>
> They can pick it up the same way though, no? Actually I think they
> usually pick it up from Greg's tree anyway :)
"Can" and "do" are different things. And you "think" you know what
others people do but do you really "know" it? Anyway your original
comment sounded a bit like "avoid sending it to the stable kernel
mailinglist otherwise unwanted things might happen". In the end this
raises the question why that stable kernel mailing still list exists or
why there is no general rule not to send mails to it.
So in the end that currently means that it is not wrong to send mail to
[email protected] but it is your opinion that it is just not really
necessary, right?
- ron
--
Ronald Wahl - [email protected] - Phone +49 375271349-0 Fax -99
Raritan Deutschland GmbH, Kornmarkt 7, 08056 Zwickau, Germany
USt-IdNr. DE813094160, Steuer-Nr. 227/117/01749
Amtsgericht Chemnitz HRB 23605
Geschäftsführung: Stuart Hopper, Ralf Ploenes
On 11/06/14 14:55, Ard Biesheuvel wrote:
> On 6 November 2014 14:50, Ronald Wahl<[email protected]> wrote:
>> On 06.11.2014 14:14, Johannes Berg wrote:
>>>
>>> On Thu, 2014-11-06 at 14:07 +0100, Ronald Wahl wrote:
>>>
>>>> But there are LTS kernels not maintained by Greg like 3.12 and 3.16. How
>>>> about these? If sending patches to stable@kernel org is optional then
>>>> it's better to kill that list or silently drop emails send to this list
>>>> so no errors are returned for this address.
>>>
>>>
>>> They can pick it up the same way though, no? Actually I think they
>>> usually pick it up from Greg's tree anyway :)
>>
>>
>> "Can" and "do" are different things. And you "think" you know what others
>> people do but do you really "know" it? Anyway your original comment sounded
>> a bit like "avoid sending it to the stable kernel mailinglist otherwise
>> unwanted things might happen". In the end this raises the question why that
>> stable kernel mailing still list exists or why there is no general rule not
>> to send mails to it.
>>
>> So in the end that currently means that it is not wrong to send mail to
>> [email protected] but it is your opinion that it is just not really
>> necessary, right?
>>
>
> You used to get an autoreply like this when cc'ing stable@ directly
>
> """
> <formletter>
>
> This is not the correct way to submit patches for inclusion in the
> stable kernel tree. Please read Documentation/stable_kernel_rules.txt
> for how to do this properly.
>
> </formletter>
> """
>
> You are only supposed to send patches to the mailing list if they are
> already upstream, i.e., if you know the upstream commit id of the
> patch.
Actually, adding the "Cc: [email protected]" tag will assure that
Greg picks it up automatically from upstream thus no need to send the
patch to stable provided it will apply to stable trees as is.
Regards,
Arend
On Thu, 2014-11-06 at 13:34 +0100, Arend van Spriel wrote:
> On 06-11-14 11:02, Johannes Berg wrote:
> > Oh, regarding Cc stable - don't actually send the mail there if you can
> > help it.
>
> What do you mean? Are you saying: It must be tagged, but 'git
> send-email' should not actually add the Cc field in the mail header.
> What is the story here? Guess I missed a memo ;-)
As far as I remember Greg just ignores the patches sent to the list and
picks them up when they hit Linus's tree instead (i.e. follows the
commit logs with like 'git log --grep "Cc: stable"') so there's no value
in actually sending out email, it's just noise.
So far it has worked for me, so I think it must be right :)
johannes
On 6 November 2014 14:50, Ronald Wahl <[email protected]> wrote:
> On 06.11.2014 14:14, Johannes Berg wrote:
>>
>> On Thu, 2014-11-06 at 14:07 +0100, Ronald Wahl wrote:
>>
>>> But there are LTS kernels not maintained by Greg like 3.12 and 3.16. How
>>> about these? If sending patches to stable@kernel org is optional then
>>> it's better to kill that list or silently drop emails send to this list
>>> so no errors are returned for this address.
>>
>>
>> They can pick it up the same way though, no? Actually I think they
>> usually pick it up from Greg's tree anyway :)
>
>
> "Can" and "do" are different things. And you "think" you know what others
> people do but do you really "know" it? Anyway your original comment sounded
> a bit like "avoid sending it to the stable kernel mailinglist otherwise
> unwanted things might happen". In the end this raises the question why that
> stable kernel mailing still list exists or why there is no general rule not
> to send mails to it.
>
> So in the end that currently means that it is not wrong to send mail to
> [email protected] but it is your opinion that it is just not really
> necessary, right?
>
You used to get an autoreply like this when cc'ing stable@ directly
"""
<formletter>
This is not the correct way to submit patches for inclusion in the
stable kernel tree. Please read Documentation/stable_kernel_rules.txt
for how to do this properly.
</formletter>
"""
You are only supposed to send patches to the mailing list if they are
already upstream, i.e., if you know the upstream commit id of the
patch.
--
Ard.
On Thu, 2014-11-06 at 14:07 +0100, Ronald Wahl wrote:
> But there are LTS kernels not maintained by Greg like 3.12 and 3.16. How
> about these? If sending patches to stable@kernel org is optional then
> it's better to kill that list or silently drop emails send to this list
> so no errors are returned for this address.
They can pick it up the same way though, no? Actually I think they
usually pick it up from Greg's tree anyway :)
johannes