* [PATCH] crypto: user - re-add size check for CRYPTO_MSG_GETALG
@ 2016-06-22 18:29 Mathias Krause
2016-06-22 19:03 ` Stephan Mueller
2016-06-23 10:43 ` Herbert Xu
0 siblings, 2 replies; 6+ messages in thread
From: Mathias Krause @ 2016-06-22 18:29 UTC (permalink / raw)
To: Herbert Xu
Cc: David S. Miller, linux-crypto, Mathias Krause, Steffen Klassert
Commit 9aa867e46565 ("crypto: user - Add CRYPTO_MSG_DELRNG")
accidentally removed the minimum size check for CRYPTO_MSG_GETALG
netlink messages. This allows userland to send a truncated
CRYPTO_MSG_GETALG message as short as a netlink header only making
crypto_report() operate on uninitialized memory by accessing data
beyond the end of the netlink message.
Fix this be re-adding the minimum required size of CRYPTO_MSG_GETALG
messages to the crypto_msg_min[] array.
Fixes: 9aa867e46565 ("crypto: user - Add CRYPTO_MSG_DELRNG")
Cc: stable@vger.kernel.org # v4.2
Signed-off-by: Mathias Krause <minipli@googlemail.com>
Cc: Steffen Klassert <steffen.klassert@secunet.com>
---
This should go on top of crypto-2.6/master.
crypto/crypto_user.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c
index 43fe85f20d57..7097a3395b25 100644
--- a/crypto/crypto_user.c
+++ b/crypto/crypto_user.c
@@ -455,6 +455,7 @@ static const int crypto_msg_min[CRYPTO_NR_MSGTYPES] = {
[CRYPTO_MSG_NEWALG - CRYPTO_MSG_BASE] = MSGSIZE(crypto_user_alg),
[CRYPTO_MSG_DELALG - CRYPTO_MSG_BASE] = MSGSIZE(crypto_user_alg),
[CRYPTO_MSG_UPDATEALG - CRYPTO_MSG_BASE] = MSGSIZE(crypto_user_alg),
+ [CRYPTO_MSG_GETALG - CRYPTO_MSG_BASE] = MSGSIZE(crypto_user_alg),
[CRYPTO_MSG_DELRNG - CRYPTO_MSG_BASE] = 0,
};
--
1.7.10.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] crypto: user - re-add size check for CRYPTO_MSG_GETALG
2016-06-22 18:29 [PATCH] crypto: user - re-add size check for CRYPTO_MSG_GETALG Mathias Krause
@ 2016-06-22 19:03 ` Stephan Mueller
2016-06-22 19:43 ` Mathias Krause
2016-06-23 10:43 ` Herbert Xu
1 sibling, 1 reply; 6+ messages in thread
From: Stephan Mueller @ 2016-06-22 19:03 UTC (permalink / raw)
To: Mathias Krause
Cc: Herbert Xu, David S. Miller, linux-crypto, Steffen Klassert
Am Mittwoch, 22. Juni 2016, 20:29:37 schrieb Mathias Krause:
Hi Mathias,
> Commit 9aa867e46565 ("crypto: user - Add CRYPTO_MSG_DELRNG")
> accidentally removed the minimum size check for CRYPTO_MSG_GETALG
> netlink messages. This allows userland to send a truncated
> CRYPTO_MSG_GETALG message as short as a netlink header only making
> crypto_report() operate on uninitialized memory by accessing data
> beyond the end of the netlink message.
>
> Fix this be re-adding the minimum required size of CRYPTO_MSG_GETALG
> messages to the crypto_msg_min[] array.
I was playing with the adding of the GETALG value as you did to track down the
issue fixed with eed1e1afd8d542d9644534c1b712599b5d680007 ("crypto: user - no
parsing of CRYPTO_MSG_GETALG") in the cryptodev-2.6 tree.
It did not occur to me that it fixes another bug. Yet, with this addition, it
would be possible to revert the patch eed1e1afd8d542d9644534c1b712599b5d680007
as your patch fixes the issue too. But my fix can also stay as it does not
hurt either.
What is your take on that?
Ciao
Stephan
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] crypto: user - re-add size check for CRYPTO_MSG_GETALG
2016-06-22 19:03 ` Stephan Mueller
@ 2016-06-22 19:43 ` Mathias Krause
0 siblings, 0 replies; 6+ messages in thread
From: Mathias Krause @ 2016-06-22 19:43 UTC (permalink / raw)
To: Stephan Mueller
Cc: Herbert Xu, David S. Miller, linux-crypto@vger.kernel.org,
Steffen Klassert
On 22 June 2016 at 21:03, Stephan Mueller <smueller@chronox.de> wrote:
> Am Mittwoch, 22. Juni 2016, 20:29:37 schrieb Mathias Krause:
>
> Hi Mathias,
>
>> Commit 9aa867e46565 ("crypto: user - Add CRYPTO_MSG_DELRNG")
>> accidentally removed the minimum size check for CRYPTO_MSG_GETALG
>> netlink messages. This allows userland to send a truncated
>> CRYPTO_MSG_GETALG message as short as a netlink header only making
>> crypto_report() operate on uninitialized memory by accessing data
>> beyond the end of the netlink message.
>>
>> Fix this be re-adding the minimum required size of CRYPTO_MSG_GETALG
>> messages to the crypto_msg_min[] array.
>
> I was playing with the adding of the GETALG value as you did to track down the
> issue fixed with eed1e1afd8d542d9644534c1b712599b5d680007 ("crypto: user - no
> parsing of CRYPTO_MSG_GETALG") in the cryptodev-2.6 tree.
Oh, I haven't noticed this commit. :D Just looked at Linus' master and
crypto-2.6/master.
> It did not occur to me that it fixes another bug. Yet, with this addition, it
> would be possible to revert the patch eed1e1afd8d542d9644534c1b712599b5d680007
> as your patch fixes the issue too. But my fix can also stay as it does not
> hurt either.
Well, it does. Commit eed1e1afd8d542d9644534c1b712599b5d680007 is
really just a workaround for the underlying issue. It does not fix the
bug of the missing minimal size check for CRYPTO_MSG_GETALG, it just
disables any further size checks for CRYPTO_MSG_GETALG. Putting my
patch on top of yours will still not fix the issue of the missing
minimal size check as your patch explicitly excludes CRYPTO_MSG_GETALG
from any size checks. So it needs to be reverted, sorry :/
> What is your take on that?
I'd say to revert eed1e1afd8d542d9644534c1b712599b5d680007 and fix the
issue for real with my patch in crypto-2.6/master, i.e. the upcoming
v4.7. That adds back the size check so userland can't play tricks with
us.
The patch already contains the Cc to stable, so the fix will
eventually end up in the LTS kernel releases used by distros, so this
regression should be fixed in a few weeks.
Regards,
Mathias
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] crypto: user - re-add size check for CRYPTO_MSG_GETALG
2016-06-22 18:29 [PATCH] crypto: user - re-add size check for CRYPTO_MSG_GETALG Mathias Krause
2016-06-22 19:03 ` Stephan Mueller
@ 2016-06-23 10:43 ` Herbert Xu
2016-06-23 14:46 ` Stephan Mueller
1 sibling, 1 reply; 6+ messages in thread
From: Herbert Xu @ 2016-06-23 10:43 UTC (permalink / raw)
To: Mathias Krause; +Cc: David S. Miller, linux-crypto, Steffen Klassert
On Wed, Jun 22, 2016 at 08:29:37PM +0200, Mathias Krause wrote:
> Commit 9aa867e46565 ("crypto: user - Add CRYPTO_MSG_DELRNG")
> accidentally removed the minimum size check for CRYPTO_MSG_GETALG
> netlink messages. This allows userland to send a truncated
> CRYPTO_MSG_GETALG message as short as a netlink header only making
> crypto_report() operate on uninitialized memory by accessing data
> beyond the end of the netlink message.
>
> Fix this be re-adding the minimum required size of CRYPTO_MSG_GETALG
> messages to the crypto_msg_min[] array.
>
> Fixes: 9aa867e46565 ("crypto: user - Add CRYPTO_MSG_DELRNG")
> Cc: stable@vger.kernel.org # v4.2
> Signed-off-by: Mathias Krause <minipli@googlemail.com>
> Cc: Steffen Klassert <steffen.klassert@secunet.com>
> ---
> This should go on top of crypto-2.6/master.
Patch applied to crypto. Thanks!
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] crypto: user - re-add size check for CRYPTO_MSG_GETALG
2016-06-23 10:43 ` Herbert Xu
@ 2016-06-23 14:46 ` Stephan Mueller
2016-06-24 1:00 ` Herbert Xu
0 siblings, 1 reply; 6+ messages in thread
From: Stephan Mueller @ 2016-06-23 14:46 UTC (permalink / raw)
To: Herbert Xu
Cc: Mathias Krause, David S. Miller, linux-crypto, Steffen Klassert
Am Donnerstag, 23. Juni 2016, 18:43:57 schrieb Herbert Xu:
Hi Herbert,
> On Wed, Jun 22, 2016 at 08:29:37PM +0200, Mathias Krause wrote:
> > Commit 9aa867e46565 ("crypto: user - Add CRYPTO_MSG_DELRNG")
> > accidentally removed the minimum size check for CRYPTO_MSG_GETALG
> > netlink messages. This allows userland to send a truncated
> > CRYPTO_MSG_GETALG message as short as a netlink header only making
> > crypto_report() operate on uninitialized memory by accessing data
> > beyond the end of the netlink message.
> >
> > Fix this be re-adding the minimum required size of CRYPTO_MSG_GETALG
> > messages to the crypto_msg_min[] array.
> >
> > Fixes: 9aa867e46565 ("crypto: user - Add CRYPTO_MSG_DELRNG")
> > Cc: stable@vger.kernel.org # v4.2
> > Signed-off-by: Mathias Krause <minipli@googlemail.com>
> > Cc: Steffen Klassert <steffen.klassert@secunet.com>
> > ---
> > This should go on top of crypto-2.6/master.
>
> Patch applied to crypto. Thanks!
Please revert my patch eed1e1afd8d542d9644534c1b712599b5d680007 as requested
by Matthias.
Ciao
Stephan
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-06-24 1:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-06-22 18:29 [PATCH] crypto: user - re-add size check for CRYPTO_MSG_GETALG Mathias Krause
2016-06-22 19:03 ` Stephan Mueller
2016-06-22 19:43 ` Mathias Krause
2016-06-23 10:43 ` Herbert Xu
2016-06-23 14:46 ` Stephan Mueller
2016-06-24 1:00 ` Herbert Xu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox