* [PATCH] crypto: x86/sha1 : Fix reads beyond the number of blocks passed
@ 2017-08-23 0:41 Megha Dey
2017-08-23 0:47 ` Greg KH
0 siblings, 1 reply; 9+ messages in thread
From: Megha Dey @ 2017-08-23 0:41 UTC (permalink / raw)
To: stable; +Cc: Megha Dey
It was reported that the sha1 AVX2 function(sha1_transform_avx2) is
reading ahead beyond its intended data, and causing a crash if the next
block is beyond page boundary:
http://marc.info/?l=linux-crypto-vger&m=149373371023377
This patch makes sure that there is no overflow for any buffer length.
It passes the tests written by Jan Stancek that revealed this problem:
https://github.com/jstancek/sha1-avx2-crash
This patch fixes reads beyond the number of blocks in the same way it
was done in commit 8861249c740fc4af9ddc5aee321eafefb960d7c6
("crypto: x86/sha1 : Fix reads beyond the number of blocks passed").
Originally-by: Ilya Albrekht <ilya.albrekht@intel.com>
Tested-by: Jan Stancek <jstancek@redhat.com>
Signed-off-by: Megha Dey <megha.dey@linux.intel.com>
Reported-by: Jan Stancek <jstancek@redhat.com>
---
arch/x86/crypto/sha1_avx2_x86_64_asm.S | 67 ++++++++++++++++++----------------
1 file changed, 36 insertions(+), 31 deletions(-)
diff --git a/arch/x86/crypto/sha1_avx2_x86_64_asm.S b/arch/x86/crypto/sha1_avx2_x86_64_asm.S
index 1cd792d..1eab79c 100644
--- a/arch/x86/crypto/sha1_avx2_x86_64_asm.S
+++ b/arch/x86/crypto/sha1_avx2_x86_64_asm.S
@@ -117,11 +117,10 @@
.set T1, REG_T1
.endm
-#define K_BASE %r8
#define HASH_PTR %r9
+#define BLOCKS_CTR %r8
#define BUFFER_PTR %r10
#define BUFFER_PTR2 %r13
-#define BUFFER_END %r11
#define PRECALC_BUF %r14
#define WK_BUF %r15
@@ -205,14 +204,14 @@
* blended AVX2 and ALU instruction scheduling
* 1 vector iteration per 8 rounds
*/
- vmovdqu ((i * 2) + PRECALC_OFFSET)(BUFFER_PTR), W_TMP
+ vmovdqu (i * 2)(BUFFER_PTR), W_TMP
.elseif ((i & 7) == 1)
- vinsertf128 $1, (((i-1) * 2)+PRECALC_OFFSET)(BUFFER_PTR2),\
+ vinsertf128 $1, ((i-1) * 2)(BUFFER_PTR2),\
WY_TMP, WY_TMP
.elseif ((i & 7) == 2)
vpshufb YMM_SHUFB_BSWAP, WY_TMP, WY
.elseif ((i & 7) == 4)
- vpaddd K_XMM(K_BASE), WY, WY_TMP
+ vpaddd K_XMM + K_XMM_AR(%rip), WY, WY_TMP
.elseif ((i & 7) == 7)
vmovdqu WY_TMP, PRECALC_WK(i&~7)
@@ -255,7 +254,7 @@
vpxor WY, WY_TMP, WY_TMP
.elseif ((i & 7) == 7)
vpxor WY_TMP2, WY_TMP, WY
- vpaddd K_XMM(K_BASE), WY, WY_TMP
+ vpaddd K_XMM + K_XMM_AR(%rip), WY, WY_TMP
vmovdqu WY_TMP, PRECALC_WK(i&~7)
PRECALC_ROTATE_WY
@@ -291,7 +290,7 @@
vpsrld $30, WY, WY
vpor WY, WY_TMP, WY
.elseif ((i & 7) == 7)
- vpaddd K_XMM(K_BASE), WY, WY_TMP
+ vpaddd K_XMM + K_XMM_AR(%rip), WY, WY_TMP
vmovdqu WY_TMP, PRECALC_WK(i&~7)
PRECALC_ROTATE_WY
@@ -446,6 +445,16 @@
.endm
+/* Add constant only if (%2 > %3) condition met (uses RTA as temp)
+ * %1 + %2 >= %3 ? %4 : 0
+ */
+.macro ADD_IF_GE a, b, c, d
+ mov \a, RTA
+ add $\d, RTA
+ cmp $\c, \b
+ cmovge RTA, \a
+.endm
+
/*
* macro implements 80 rounds of SHA-1, for multiple blocks with s/w pipelining
*/
@@ -463,13 +472,16 @@
lea (2*4*80+32)(%rsp), WK_BUF
# Precalc WK for first 2 blocks
- PRECALC_OFFSET = 0
+ ADD_IF_GE BUFFER_PTR2, BLOCKS_CTR, 2, 64
.set i, 0
.rept 160
PRECALC i
.set i, i + 1
.endr
- PRECALC_OFFSET = 128
+
+ /* Go to next block if needed */
+ ADD_IF_GE BUFFER_PTR, BLOCKS_CTR, 3, 128
+ ADD_IF_GE BUFFER_PTR2, BLOCKS_CTR, 4, 128
xchg WK_BUF, PRECALC_BUF
.align 32
@@ -479,8 +491,8 @@ _loop:
* we use K_BASE value as a signal of a last block,
* it is set below by: cmovae BUFFER_PTR, K_BASE
*/
- cmp K_BASE, BUFFER_PTR
- jne _begin
+ test BLOCKS_CTR, BLOCKS_CTR
+ jnz _begin
.align 32
jmp _end
.align 32
@@ -512,10 +524,10 @@ _loop0:
.set j, j+2
.endr
- add $(2*64), BUFFER_PTR /* move to next odd-64-byte block */
- cmp BUFFER_END, BUFFER_PTR /* is current block the last one? */
- cmovae K_BASE, BUFFER_PTR /* signal the last iteration smartly */
-
+ /* Update Counter */
+ sub $1, BLOCKS_CTR
+ /* Move to the next block only if needed*/
+ ADD_IF_GE BUFFER_PTR, BLOCKS_CTR, 4, 128
/*
* rounds
* 60,62,64,66,68
@@ -532,8 +544,8 @@ _loop0:
UPDATE_HASH 12(HASH_PTR), D
UPDATE_HASH 16(HASH_PTR), E
- cmp K_BASE, BUFFER_PTR /* is current block the last one? */
- je _loop
+ test BLOCKS_CTR, BLOCKS_CTR
+ jz _loop
mov TB, B
@@ -575,10 +587,10 @@ _loop2:
.set j, j+2
.endr
- add $(2*64), BUFFER_PTR2 /* move to next even-64-byte block */
-
- cmp BUFFER_END, BUFFER_PTR2 /* is current block the last one */
- cmovae K_BASE, BUFFER_PTR /* signal the last iteration smartly */
+ /* update counter */
+ sub $1, BLOCKS_CTR
+ /* Move to the next block only if needed*/
+ ADD_IF_GE BUFFER_PTR2, BLOCKS_CTR, 4, 128
jmp _loop3
_loop3:
@@ -641,19 +653,12 @@ _loop3:
avx2_zeroupper
- lea K_XMM_AR(%rip), K_BASE
-
+ /* Setup initial values */
mov CTX, HASH_PTR
mov BUF, BUFFER_PTR
- lea 64(BUF), BUFFER_PTR2
-
- shl $6, CNT /* mul by 64 */
- add BUF, CNT
- add $64, CNT
- mov CNT, BUFFER_END
- cmp BUFFER_END, BUFFER_PTR2
- cmovae K_BASE, BUFFER_PTR2
+ mov BUF, BUFFER_PTR2
+ mov CNT, BLOCKS_CTR
xmm_mov BSWAP_SHUFB_CTL(%rip), YMM_SHUFB_BSWAP
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] crypto: x86/sha1 : Fix reads beyond the number of blocks passed
2017-08-23 0:41 [PATCH] crypto: x86/sha1 : Fix reads beyond the number of blocks passed Megha Dey
@ 2017-08-23 0:47 ` Greg KH
2017-08-29 17:08 ` Megha Dey
0 siblings, 1 reply; 9+ messages in thread
From: Greg KH @ 2017-08-23 0:47 UTC (permalink / raw)
To: Megha Dey; +Cc: stable
On Tue, Aug 22, 2017 at 05:41:03PM -0700, Megha Dey wrote:
> It was reported that the sha1 AVX2 function(sha1_transform_avx2) is
> reading ahead beyond its intended data, and causing a crash if the next
> block is beyond page boundary:
> http://marc.info/?l=linux-crypto-vger&m=149373371023377
>
> This patch makes sure that there is no overflow for any buffer length.
>
> It passes the tests written by Jan Stancek that revealed this problem:
> https://github.com/jstancek/sha1-avx2-crash
>
> This patch fixes reads beyond the number of blocks in the same way it
> was done in commit 8861249c740fc4af9ddc5aee321eafefb960d7c6
> ("crypto: x86/sha1 : Fix reads beyond the number of blocks passed").
So all you really want is that specific commit added to the stable
kernels? If so, what kernel tree(s)?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] crypto: x86/sha1 : Fix reads beyond the number of blocks passed
2017-08-23 0:47 ` Greg KH
@ 2017-08-29 17:08 ` Megha Dey
2017-08-31 6:03 ` Greg KH
0 siblings, 1 reply; 9+ messages in thread
From: Megha Dey @ 2017-08-29 17:08 UTC (permalink / raw)
To: Greg KH; +Cc: stable
On Tue, 2017-08-22 at 17:47 -0700, Greg KH wrote:
> On Tue, Aug 22, 2017 at 05:41:03PM -0700, Megha Dey wrote:
> > It was reported that the sha1 AVX2 function(sha1_transform_avx2) is
> > reading ahead beyond its intended data, and causing a crash if the next
> > block is beyond page boundary:
> > http://marc.info/?l=linux-crypto-vger&m=149373371023377
> >
> > This patch makes sure that there is no overflow for any buffer length.
> >
> > It passes the tests written by Jan Stancek that revealed this problem:
> > https://github.com/jstancek/sha1-avx2-crash
> >
> > This patch fixes reads beyond the number of blocks in the same way it
> > was done in commit 8861249c740fc4af9ddc5aee321eafefb960d7c6
> > ("crypto: x86/sha1 : Fix reads beyond the number of blocks passed").
>
> So all you really want is that specific commit added to the stable
> kernels? If so, what kernel tree(s)?
Hi Greg,
The commit 8861249c740fc4af9ddc5aee321eafefb960d7c6 present in the
mainline kernel does not apply cleanly to the stable kernel tree. Hence,
I have submitted this patch with some minor changes for the stable tree.
I am not sure what you mean by which kernel trees.
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] crypto: x86/sha1 : Fix reads beyond the number of blocks passed
2017-08-29 17:08 ` Megha Dey
@ 2017-08-31 6:03 ` Greg KH
2017-08-31 6:06 ` Greg KH
0 siblings, 1 reply; 9+ messages in thread
From: Greg KH @ 2017-08-31 6:03 UTC (permalink / raw)
To: Megha Dey; +Cc: stable
On Tue, Aug 29, 2017 at 10:08:31AM -0700, Megha Dey wrote:
> On Tue, 2017-08-22 at 17:47 -0700, Greg KH wrote:
> > On Tue, Aug 22, 2017 at 05:41:03PM -0700, Megha Dey wrote:
> > > It was reported that the sha1 AVX2 function(sha1_transform_avx2) is
> > > reading ahead beyond its intended data, and causing a crash if the next
> > > block is beyond page boundary:
> > > http://marc.info/?l=linux-crypto-vger&m=149373371023377
> > >
> > > This patch makes sure that there is no overflow for any buffer length.
> > >
> > > It passes the tests written by Jan Stancek that revealed this problem:
> > > https://github.com/jstancek/sha1-avx2-crash
> > >
> > > This patch fixes reads beyond the number of blocks in the same way it
> > > was done in commit 8861249c740fc4af9ddc5aee321eafefb960d7c6
> > > ("crypto: x86/sha1 : Fix reads beyond the number of blocks passed").
> >
> > So all you really want is that specific commit added to the stable
> > kernels? If so, what kernel tree(s)?
>
> Hi Greg,
> The commit 8861249c740fc4af9ddc5aee321eafefb960d7c6 present in the
> mainline kernel does not apply cleanly to the stable kernel tree. Hence,
> I have submitted this patch with some minor changes for the stable tree.
>
> I am not sure what you mean by which kernel trees.
There are lots of stable kernel trees being maintained at the moment,
see:
https://www.kernel.org/category/releases.html
I was asking if any of those are applicable for this patch as well.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] crypto: x86/sha1 : Fix reads beyond the number of blocks passed
2017-08-31 6:03 ` Greg KH
@ 2017-08-31 6:06 ` Greg KH
2017-08-31 17:35 ` Megha Dey
0 siblings, 1 reply; 9+ messages in thread
From: Greg KH @ 2017-08-31 6:06 UTC (permalink / raw)
To: Megha Dey; +Cc: stable
On Thu, Aug 31, 2017 at 08:03:59AM +0200, Greg KH wrote:
> On Tue, Aug 29, 2017 at 10:08:31AM -0700, Megha Dey wrote:
> > On Tue, 2017-08-22 at 17:47 -0700, Greg KH wrote:
> > > On Tue, Aug 22, 2017 at 05:41:03PM -0700, Megha Dey wrote:
> > > > It was reported that the sha1 AVX2 function(sha1_transform_avx2) is
> > > > reading ahead beyond its intended data, and causing a crash if the next
> > > > block is beyond page boundary:
> > > > http://marc.info/?l=linux-crypto-vger&m=149373371023377
> > > >
> > > > This patch makes sure that there is no overflow for any buffer length.
> > > >
> > > > It passes the tests written by Jan Stancek that revealed this problem:
> > > > https://github.com/jstancek/sha1-avx2-crash
> > > >
> > > > This patch fixes reads beyond the number of blocks in the same way it
> > > > was done in commit 8861249c740fc4af9ddc5aee321eafefb960d7c6
> > > > ("crypto: x86/sha1 : Fix reads beyond the number of blocks passed").
> > >
> > > So all you really want is that specific commit added to the stable
> > > kernels? If so, what kernel tree(s)?
> >
> > Hi Greg,
> > The commit 8861249c740fc4af9ddc5aee321eafefb960d7c6 present in the
> > mainline kernel does not apply cleanly to the stable kernel tree. Hence,
> > I have submitted this patch with some minor changes for the stable tree.
> >
> > I am not sure what you mean by which kernel trees.
>
> There are lots of stable kernel trees being maintained at the moment,
> see:
> https://www.kernel.org/category/releases.html
>
> I was asking if any of those are applicable for this patch as well.
Even more confusing, commit 8861249c740fc4af9ddc5aee321eafefb960d7c6 is
already in the stable kernel releases. It showed up in 4.4.84, 4.9.45,
and 4.12.9. You should have gotten notifications about all of these.
So I don't really understand what needs to be done here.
totally confused,
greg k-h
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] crypto: x86/sha1 : Fix reads beyond the number of blocks passed
2017-08-31 6:06 ` Greg KH
@ 2017-08-31 17:35 ` Megha Dey
2017-09-04 9:34 ` Greg KH
0 siblings, 1 reply; 9+ messages in thread
From: Megha Dey @ 2017-08-31 17:35 UTC (permalink / raw)
To: Greg KH; +Cc: stable
On Thu, 2017-08-31 at 08:06 +0200, Greg KH wrote:
> On Thu, Aug 31, 2017 at 08:03:59AM +0200, Greg KH wrote:
> > On Tue, Aug 29, 2017 at 10:08:31AM -0700, Megha Dey wrote:
> > > On Tue, 2017-08-22 at 17:47 -0700, Greg KH wrote:
> > > > On Tue, Aug 22, 2017 at 05:41:03PM -0700, Megha Dey wrote:
> > > > > It was reported that the sha1 AVX2 function(sha1_transform_avx2) is
> > > > > reading ahead beyond its intended data, and causing a crash if the next
> > > > > block is beyond page boundary:
> > > > > http://marc.info/?l=linux-crypto-vger&m=149373371023377
> > > > >
> > > > > This patch makes sure that there is no overflow for any buffer length.
> > > > >
> > > > > It passes the tests written by Jan Stancek that revealed this problem:
> > > > > https://github.com/jstancek/sha1-avx2-crash
> > > > >
> > > > > This patch fixes reads beyond the number of blocks in the same way it
> > > > > was done in commit 8861249c740fc4af9ddc5aee321eafefb960d7c6
> > > > > ("crypto: x86/sha1 : Fix reads beyond the number of blocks passed").
> > > >
> > > > So all you really want is that specific commit added to the stable
> > > > kernels? If so, what kernel tree(s)?
> > >
> > > Hi Greg,
> > > The commit 8861249c740fc4af9ddc5aee321eafefb960d7c6 present in the
> > > mainline kernel does not apply cleanly to the stable kernel tree. Hence,
> > > I have submitted this patch with some minor changes for the stable tree.
> > >
> > > I am not sure what you mean by which kernel trees.
> >
> > There are lots of stable kernel trees being maintained at the moment,
> > see:
> > https://www.kernel.org/category/releases.html
> >
> > I was asking if any of those are applicable for this patch as well.
>
> Even more confusing, commit 8861249c740fc4af9ddc5aee321eafefb960d7c6 is
> already in the stable kernel releases. It showed up in 4.4.84, 4.9.45,
> and 4.12.9. You should have gotten notifications about all of these.
>
> So I don't really understand what needs to be done here.
>
> totally confused,
Hi Greg,
Sorry for the confusion.
I had got an email from you:
"The patch below does not apply to the 3.18-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>."
This new patch applies cleanly on the 3.18 stable kernel unlike commit
8861249. So to answer your question, this patch needs to be added to the
3.18 stable kernel.
-Megha
>
> greg k-h
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] crypto: x86/sha1 : Fix reads beyond the number of blocks passed
2017-08-31 17:35 ` Megha Dey
@ 2017-09-04 9:34 ` Greg KH
2017-09-18 6:42 ` Greg KH
0 siblings, 1 reply; 9+ messages in thread
From: Greg KH @ 2017-09-04 9:34 UTC (permalink / raw)
To: Megha Dey; +Cc: stable
On Thu, Aug 31, 2017 at 10:35:31AM -0700, Megha Dey wrote:
> On Thu, 2017-08-31 at 08:06 +0200, Greg KH wrote:
> > On Thu, Aug 31, 2017 at 08:03:59AM +0200, Greg KH wrote:
> > > On Tue, Aug 29, 2017 at 10:08:31AM -0700, Megha Dey wrote:
> > > > On Tue, 2017-08-22 at 17:47 -0700, Greg KH wrote:
> > > > > On Tue, Aug 22, 2017 at 05:41:03PM -0700, Megha Dey wrote:
> > > > > > It was reported that the sha1 AVX2 function(sha1_transform_avx2) is
> > > > > > reading ahead beyond its intended data, and causing a crash if the next
> > > > > > block is beyond page boundary:
> > > > > > http://marc.info/?l=linux-crypto-vger&m=149373371023377
> > > > > >
> > > > > > This patch makes sure that there is no overflow for any buffer length.
> > > > > >
> > > > > > It passes the tests written by Jan Stancek that revealed this problem:
> > > > > > https://github.com/jstancek/sha1-avx2-crash
> > > > > >
> > > > > > This patch fixes reads beyond the number of blocks in the same way it
> > > > > > was done in commit 8861249c740fc4af9ddc5aee321eafefb960d7c6
> > > > > > ("crypto: x86/sha1 : Fix reads beyond the number of blocks passed").
> > > > >
> > > > > So all you really want is that specific commit added to the stable
> > > > > kernels? If so, what kernel tree(s)?
> > > >
> > > > Hi Greg,
> > > > The commit 8861249c740fc4af9ddc5aee321eafefb960d7c6 present in the
> > > > mainline kernel does not apply cleanly to the stable kernel tree. Hence,
> > > > I have submitted this patch with some minor changes for the stable tree.
> > > >
> > > > I am not sure what you mean by which kernel trees.
> > >
> > > There are lots of stable kernel trees being maintained at the moment,
> > > see:
> > > https://www.kernel.org/category/releases.html
> > >
> > > I was asking if any of those are applicable for this patch as well.
> >
> > Even more confusing, commit 8861249c740fc4af9ddc5aee321eafefb960d7c6 is
> > already in the stable kernel releases. It showed up in 4.4.84, 4.9.45,
> > and 4.12.9. You should have gotten notifications about all of these.
> >
> > So I don't really understand what needs to be done here.
> >
> > totally confused,
> Hi Greg,
>
> Sorry for the confusion.
>
> I had got an email from you:
> "The patch below does not apply to the 3.18-stable tree.
> If someone wants it applied there, or to any other stable or longterm
> tree, then please email the backport, including the original git commit
> id to <stable@vger.kernel.org>."
>
> This new patch applies cleanly on the 3.18 stable kernel unlike commit
> 8861249. So to answer your question, this patch needs to be added to the
> 3.18 stable kernel.
Ok, but commit 8861249 modifies a lot of files, while your "backport"
does not do that at all.
Again, I still do not know what is going on here, or what exactly you
want. How about a backport of the original patch, to the 3.18-stable
kernel tree, that is what I think is needed, correct?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] crypto: x86/sha1 : Fix reads beyond the number of blocks passed
2017-09-04 9:34 ` Greg KH
@ 2017-09-18 6:42 ` Greg KH
0 siblings, 0 replies; 9+ messages in thread
From: Greg KH @ 2017-09-18 6:42 UTC (permalink / raw)
To: Megha Dey; +Cc: stable
On Mon, Sep 04, 2017 at 11:34:24AM +0200, Greg KH wrote:
> On Thu, Aug 31, 2017 at 10:35:31AM -0700, Megha Dey wrote:
> > On Thu, 2017-08-31 at 08:06 +0200, Greg KH wrote:
> > > On Thu, Aug 31, 2017 at 08:03:59AM +0200, Greg KH wrote:
> > > > On Tue, Aug 29, 2017 at 10:08:31AM -0700, Megha Dey wrote:
> > > > > On Tue, 2017-08-22 at 17:47 -0700, Greg KH wrote:
> > > > > > On Tue, Aug 22, 2017 at 05:41:03PM -0700, Megha Dey wrote:
> > > > > > > It was reported that the sha1 AVX2 function(sha1_transform_avx2) is
> > > > > > > reading ahead beyond its intended data, and causing a crash if the next
> > > > > > > block is beyond page boundary:
> > > > > > > http://marc.info/?l=linux-crypto-vger&m=149373371023377
> > > > > > >
> > > > > > > This patch makes sure that there is no overflow for any buffer length.
> > > > > > >
> > > > > > > It passes the tests written by Jan Stancek that revealed this problem:
> > > > > > > https://github.com/jstancek/sha1-avx2-crash
> > > > > > >
> > > > > > > This patch fixes reads beyond the number of blocks in the same way it
> > > > > > > was done in commit 8861249c740fc4af9ddc5aee321eafefb960d7c6
> > > > > > > ("crypto: x86/sha1 : Fix reads beyond the number of blocks passed").
> > > > > >
> > > > > > So all you really want is that specific commit added to the stable
> > > > > > kernels? If so, what kernel tree(s)?
> > > > >
> > > > > Hi Greg,
> > > > > The commit 8861249c740fc4af9ddc5aee321eafefb960d7c6 present in the
> > > > > mainline kernel does not apply cleanly to the stable kernel tree. Hence,
> > > > > I have submitted this patch with some minor changes for the stable tree.
> > > > >
> > > > > I am not sure what you mean by which kernel trees.
> > > >
> > > > There are lots of stable kernel trees being maintained at the moment,
> > > > see:
> > > > https://www.kernel.org/category/releases.html
> > > >
> > > > I was asking if any of those are applicable for this patch as well.
> > >
> > > Even more confusing, commit 8861249c740fc4af9ddc5aee321eafefb960d7c6 is
> > > already in the stable kernel releases. It showed up in 4.4.84, 4.9.45,
> > > and 4.12.9. You should have gotten notifications about all of these.
> > >
> > > So I don't really understand what needs to be done here.
> > >
> > > totally confused,
> > Hi Greg,
> >
> > Sorry for the confusion.
> >
> > I had got an email from you:
> > "The patch below does not apply to the 3.18-stable tree.
> > If someone wants it applied there, or to any other stable or longterm
> > tree, then please email the backport, including the original git commit
> > id to <stable@vger.kernel.org>."
> >
> > This new patch applies cleanly on the 3.18 stable kernel unlike commit
> > 8861249. So to answer your question, this patch needs to be added to the
> > 3.18 stable kernel.
>
> Ok, but commit 8861249 modifies a lot of files, while your "backport"
> does not do that at all.
>
> Again, I still do not know what is going on here, or what exactly you
> want. How about a backport of the original patch, to the 3.18-stable
> kernel tree, that is what I think is needed, correct?
Dropping this email thread from my "todo" stable queue due to a lack of
response, if you still want this applied, please fix up and make it
obvious what I need to do here.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] crypto: x86/sha1 : Fix reads beyond the number of blocks passed
@ 2017-08-02 0:03 Megha Dey
0 siblings, 0 replies; 9+ messages in thread
From: Megha Dey @ 2017-08-02 0:03 UTC (permalink / raw)
To: herbert
Cc: tim.c.chen, davem, linux-crypto, linux-kernel, jstancek,
megha.dey, Megha Dey
It was reported that the sha1 AVX2 function(sha1_transform_avx2) is
reading ahead beyond its intended data, and causing a crash if the next
block is beyond page boundary:
http://marc.info/?l=linux-crypto-vger&m=149373371023377
This patch makes sure that there is no overflow for any buffer length.
It passes the tests written by Jan Stancek that revealed this problem:
https://github.com/jstancek/sha1-avx2-crash
Jan, can you verify this fix?
Herbert, can you re-enable sha1-avx2 once Jan has checked it out and
revert commit b82ce24426a4071da9529d726057e4e642948667 ?
Signed-off-by: Megha Dey <megha.dey@linux.intel.com>
Reported-by: Jan Stancek <jstancek@redhat.com>
---
arch/x86/crypto/sha1_avx2_x86_64_asm.S | 67 ++++++++++++++++++----------------
1 file changed, 36 insertions(+), 31 deletions(-)
diff --git a/arch/x86/crypto/sha1_avx2_x86_64_asm.S b/arch/x86/crypto/sha1_avx2_x86_64_asm.S
index 1cd792d..1eab79c 100644
--- a/arch/x86/crypto/sha1_avx2_x86_64_asm.S
+++ b/arch/x86/crypto/sha1_avx2_x86_64_asm.S
@@ -117,11 +117,10 @@
.set T1, REG_T1
.endm
-#define K_BASE %r8
#define HASH_PTR %r9
+#define BLOCKS_CTR %r8
#define BUFFER_PTR %r10
#define BUFFER_PTR2 %r13
-#define BUFFER_END %r11
#define PRECALC_BUF %r14
#define WK_BUF %r15
@@ -205,14 +204,14 @@
* blended AVX2 and ALU instruction scheduling
* 1 vector iteration per 8 rounds
*/
- vmovdqu ((i * 2) + PRECALC_OFFSET)(BUFFER_PTR), W_TMP
+ vmovdqu (i * 2)(BUFFER_PTR), W_TMP
.elseif ((i & 7) == 1)
- vinsertf128 $1, (((i-1) * 2)+PRECALC_OFFSET)(BUFFER_PTR2),\
+ vinsertf128 $1, ((i-1) * 2)(BUFFER_PTR2),\
WY_TMP, WY_TMP
.elseif ((i & 7) == 2)
vpshufb YMM_SHUFB_BSWAP, WY_TMP, WY
.elseif ((i & 7) == 4)
- vpaddd K_XMM(K_BASE), WY, WY_TMP
+ vpaddd K_XMM + K_XMM_AR(%rip), WY, WY_TMP
.elseif ((i & 7) == 7)
vmovdqu WY_TMP, PRECALC_WK(i&~7)
@@ -255,7 +254,7 @@
vpxor WY, WY_TMP, WY_TMP
.elseif ((i & 7) == 7)
vpxor WY_TMP2, WY_TMP, WY
- vpaddd K_XMM(K_BASE), WY, WY_TMP
+ vpaddd K_XMM + K_XMM_AR(%rip), WY, WY_TMP
vmovdqu WY_TMP, PRECALC_WK(i&~7)
PRECALC_ROTATE_WY
@@ -291,7 +290,7 @@
vpsrld $30, WY, WY
vpor WY, WY_TMP, WY
.elseif ((i & 7) == 7)
- vpaddd K_XMM(K_BASE), WY, WY_TMP
+ vpaddd K_XMM + K_XMM_AR(%rip), WY, WY_TMP
vmovdqu WY_TMP, PRECALC_WK(i&~7)
PRECALC_ROTATE_WY
@@ -446,6 +445,16 @@
.endm
+/* Add constant only if (%2 > %3) condition met (uses RTA as temp)
+ * %1 + %2 >= %3 ? %4 : 0
+ */
+.macro ADD_IF_GE a, b, c, d
+ mov \a, RTA
+ add $\d, RTA
+ cmp $\c, \b
+ cmovge RTA, \a
+.endm
+
/*
* macro implements 80 rounds of SHA-1, for multiple blocks with s/w pipelining
*/
@@ -463,13 +472,16 @@
lea (2*4*80+32)(%rsp), WK_BUF
# Precalc WK for first 2 blocks
- PRECALC_OFFSET = 0
+ ADD_IF_GE BUFFER_PTR2, BLOCKS_CTR, 2, 64
.set i, 0
.rept 160
PRECALC i
.set i, i + 1
.endr
- PRECALC_OFFSET = 128
+
+ /* Go to next block if needed */
+ ADD_IF_GE BUFFER_PTR, BLOCKS_CTR, 3, 128
+ ADD_IF_GE BUFFER_PTR2, BLOCKS_CTR, 4, 128
xchg WK_BUF, PRECALC_BUF
.align 32
@@ -479,8 +491,8 @@ _loop:
* we use K_BASE value as a signal of a last block,
* it is set below by: cmovae BUFFER_PTR, K_BASE
*/
- cmp K_BASE, BUFFER_PTR
- jne _begin
+ test BLOCKS_CTR, BLOCKS_CTR
+ jnz _begin
.align 32
jmp _end
.align 32
@@ -512,10 +524,10 @@ _loop0:
.set j, j+2
.endr
- add $(2*64), BUFFER_PTR /* move to next odd-64-byte block */
- cmp BUFFER_END, BUFFER_PTR /* is current block the last one? */
- cmovae K_BASE, BUFFER_PTR /* signal the last iteration smartly */
-
+ /* Update Counter */
+ sub $1, BLOCKS_CTR
+ /* Move to the next block only if needed*/
+ ADD_IF_GE BUFFER_PTR, BLOCKS_CTR, 4, 128
/*
* rounds
* 60,62,64,66,68
@@ -532,8 +544,8 @@ _loop0:
UPDATE_HASH 12(HASH_PTR), D
UPDATE_HASH 16(HASH_PTR), E
- cmp K_BASE, BUFFER_PTR /* is current block the last one? */
- je _loop
+ test BLOCKS_CTR, BLOCKS_CTR
+ jz _loop
mov TB, B
@@ -575,10 +587,10 @@ _loop2:
.set j, j+2
.endr
- add $(2*64), BUFFER_PTR2 /* move to next even-64-byte block */
-
- cmp BUFFER_END, BUFFER_PTR2 /* is current block the last one */
- cmovae K_BASE, BUFFER_PTR /* signal the last iteration smartly */
+ /* update counter */
+ sub $1, BLOCKS_CTR
+ /* Move to the next block only if needed*/
+ ADD_IF_GE BUFFER_PTR2, BLOCKS_CTR, 4, 128
jmp _loop3
_loop3:
@@ -641,19 +653,12 @@ _loop3:
avx2_zeroupper
- lea K_XMM_AR(%rip), K_BASE
-
+ /* Setup initial values */
mov CTX, HASH_PTR
mov BUF, BUFFER_PTR
- lea 64(BUF), BUFFER_PTR2
-
- shl $6, CNT /* mul by 64 */
- add BUF, CNT
- add $64, CNT
- mov CNT, BUFFER_END
- cmp BUFFER_END, BUFFER_PTR2
- cmovae K_BASE, BUFFER_PTR2
+ mov BUF, BUFFER_PTR2
+ mov CNT, BLOCKS_CTR
xmm_mov BSWAP_SHUFB_CTL(%rip), YMM_SHUFB_BSWAP
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-09-18 6:42 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-23 0:41 [PATCH] crypto: x86/sha1 : Fix reads beyond the number of blocks passed Megha Dey
2017-08-23 0:47 ` Greg KH
2017-08-29 17:08 ` Megha Dey
2017-08-31 6:03 ` Greg KH
2017-08-31 6:06 ` Greg KH
2017-08-31 17:35 ` Megha Dey
2017-09-04 9:34 ` Greg KH
2017-09-18 6:42 ` Greg KH
-- strict thread matches above, loose matches on Subject: below --
2017-08-02 0:03 Megha Dey
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.