diff mbox

[Lucid,CVE-2014-8709] mac80211: fix fragmentation code, particularly for encryption

Message ID 1416831378-3994-1-git-send-email-luis.henriques@canonical.com
State New
Headers show

Commit Message

Luis Henriques Nov. 24, 2014, 12:16 p.m. UTC
From: Johannes Berg <johannes.berg@intel.com>

The "new" fragmentation code (since my rewrite almost 5 years ago)
erroneously sets skb->len rather than using skb_trim() to adjust
the length of the first fragment after copying out all the others.
This leaves the skb tail pointer pointing to after where the data
originally ended, and thus causes the encryption MIC to be written
at that point, rather than where it belongs: immediately after the
data.

The impact of this is that if software encryption is done, then
 a) encryption doesn't work for the first fragment, the connection
    becomes unusable as the first fragment will never be properly
    verified at the receiver, the MIC is practically guaranteed to
    be wrong
 b) we leak up to 8 bytes of plaintext (!) of the packet out into
    the air

This is only mitigated by the fact that many devices are capable
of doing encryption in hardware, in which case this can't happen
as the tail pointer is irrelevant in that case. Additionally,
fragmentation is not used very frequently and would normally have
to be configured manually.

Fix this by using skb_trim() properly.

Cc: stable@vger.kernel.org
Fixes: 2de8e0d999b8 ("mac80211: rewrite fragmentation")
Reported-by: Jouni Malinen <j@w1.fi>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
(backported from commit 338f977f4eb441e69bb9a46eaa0ac715c931a67f)
CVE-2014-8709
BugLink: http://bugs.launchpad.net/bugs/1392013
Signed-off-by: Luis Henriques <luis.henriques@canonical.com>
---
 net/mac80211/tx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Andy Whitcroft Nov. 24, 2014, 12:54 p.m. UTC | #1
On Mon, Nov 24, 2014 at 12:16:18PM +0000, Luis Henriques wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> The "new" fragmentation code (since my rewrite almost 5 years ago)
> erroneously sets skb->len rather than using skb_trim() to adjust
> the length of the first fragment after copying out all the others.
> This leaves the skb tail pointer pointing to after where the data
> originally ended, and thus causes the encryption MIC to be written
> at that point, rather than where it belongs: immediately after the
> data.
> 
> The impact of this is that if software encryption is done, then
>  a) encryption doesn't work for the first fragment, the connection
>     becomes unusable as the first fragment will never be properly
>     verified at the receiver, the MIC is practically guaranteed to
>     be wrong
>  b) we leak up to 8 bytes of plaintext (!) of the packet out into
>     the air
> 
> This is only mitigated by the fact that many devices are capable
> of doing encryption in hardware, in which case this can't happen
> as the tail pointer is irrelevant in that case. Additionally,
> fragmentation is not used very frequently and would normally have
> to be configured manually.
> 
> Fix this by using skb_trim() properly.
> 
> Cc: stable@vger.kernel.org
> Fixes: 2de8e0d999b8 ("mac80211: rewrite fragmentation")
> Reported-by: Jouni Malinen <j@w1.fi>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> (backported from commit 338f977f4eb441e69bb9a46eaa0ac715c931a67f)
> CVE-2014-8709
> BugLink: http://bugs.launchpad.net/bugs/1392013
> Signed-off-by: Luis Henriques <luis.henriques@canonical.com>
> ---
>  net/mac80211/tx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> index b1d79046257b..687fc8ec71ba 100644
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -770,7 +770,7 @@ static int ieee80211_fragment(struct ieee80211_local *local,
>  		pos += fraglen;
>  	}
>  
> -	skb->len = hdrlen + per_fragm;
> +	skb_trim(skb, hdrlen + per_fragm);
>  	return 0;
>  }

Looks to do what is claimed, is simple, self-contained, therefore:

Acked-by: Andy Whitcroft <apw@canonical.com>

-apw
Stefan Bader Nov. 24, 2014, 1:02 p.m. UTC | #2

Andy Whitcroft Nov. 24, 2014, 1:22 p.m. UTC | #3
Applied to Lucid.

-apw
diff mbox

Patch

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index b1d79046257b..687fc8ec71ba 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -770,7 +770,7 @@  static int ieee80211_fragment(struct ieee80211_local *local,
 		pos += fraglen;
 	}
 
-	skb->len = hdrlen + per_fragm;
+	skb_trim(skb, hdrlen + per_fragm);
 	return 0;
 }