netdev - Re: [PATCH net-next] tcp: md5: do not use stack storage in crypto operations

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1467040961.6850.155.camel@edumazet-glaptop3.roam.corp.google.com>
Date:	Mon, 27 Jun 2016 17:22:41 +0200
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	David Miller <davem@...emloft.net>
Cc:	luto@...capital.net, herbert@...dor.apana.org.au,
	netdev@...r.kernel.org
Subject: Re: [PATCH net-next] tcp: md5: do not use stack storage in crypto
 operations

On Mon, 2016-06-27 at 10:51 -0400, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@...il.com>
> Date: Sat, 25 Jun 2016 18:09:35 +0200
> 
> > From: Eric Dumazet <edumazet@...gle.com>
> > 
> > Some arches have virtually mapped kernel stacks, or will soon have.
> > 
> > tcp_md5_hash_header() uses an automatic variable to copy tcp header
> > before mangling th->check and calling crypto function, which might
> > be problematic on such arches.
> > 
> > So use percpu storage as we already do for the pseudo header,
> > and reduce number of crypto functions calls, as these headers
> > are ridiculously small.
> > 
> > Signed-off-by: Eric Dumazet <edumazet@...gle.com>
> > Reported-by: Andy Lutomirski <luto@...capital.net>
> > ---
> > I am not sure the md5 crypto functions have a problem with data backed
> > in virtually mapped stacks, but this patch seems to remove the doubt.
> 
> In a non-SMP build this storage will be in the kernel image itself.
> 
> And this violates the crypto layer's requirements.
> 
> It has to be memory taken from the page allocator or kmalloc.  So
> before vmalloc'd stacks, the current code is perfectly fine.  And
> with vmalloc'd stacks this patch trades one bug for another.

OK, so lets simply use kmalloc() allocated storage instead of percpu.