crypto: DRBG - guard uninstantion by lock

Commit Message

Stephan Mueller April 8, 2018, 7:07 p.m. UTC

Am Sonntag, 8. April 2018, 17:41:17 CEST schrieb Dmitry Vyukov:

Hi Dmitry,
> 
> Hi,
> 
> Here is config and kernel commit:
> https://groups.google.com/d/msg/syzkaller-bugs/PINYyzoaG1s/ntZPOZdcCAAJ
> You can also find compiler and image here if necessary:
> https://github.com/google/syzkaller/blob/master/docs/syzbot.md
> 
> And note that the program needs to be compiled with -m32. The bugs is
> probably not-compat specific, but the program injects fault into a
> particular malloc invocation and maybe malloc numbering is affected by
> compat path.

I am unable to reproduce the issue. But since you mention that you induce errors, I could see that the unlocking of the DRBG context is too soon.

Can you please check whether the attached patch fixes the issue?

Thanks

---8<---

In the error code path, the uninstantiation must be guarded by a lock to
ensure that the modification of the context is fully atomic.

Signed-off-by: Stephan Mueller <[e-mail]>
Reported-by: syzkaller
---
 crypto/drbg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Theodore Ts'o April 8, 2018, 10:46 p.m. UTC | #1

On Sun, Apr 08, 2018 at 09:07:03PM +0200, Stephan Müller wrote:
> Can you please check whether the attached patch fixes the issue?
> 

Stephan,

FYI, if you incude in your e-mail "#syz test <GIT URL> <BRANCH>" as
the first line of your patch and the syzbot e-mail is cc'ed, the
syzbot will automatically apply the patch in the e-mail against the
git tree/branch specified in the "#syz test" line, and then try to see
if the problem it discovered still reproduces --- and then send you
e-mail one way or another.

So the syzbot will run while the patch goes through the normal e-mail
review process, which is kind of neat.  :-)

Cheers,

					- Ted

Stephan Mueller April 9, 2018, 5:40 a.m. UTC | #2

Am Montag, 9. April 2018, 00:46:03 CEST schrieb Theodore Y. Ts'o:

Hi Theodore,
> 
> So the syzbot will run while the patch goes through the normal e-mail
> review process, which is kind of neat.  :-)

Thank you very much for the hint. That is a neat feature indeed.

As I came late to the party and I missed the original mails, I am wondering 
about which GIT repo was used and which branch of it. With that, I would be 
happy to resubmit with the test line.

Ciao
Stephan

Patch

diff --git a/crypto/drbg.c b/crypto/drbg.c
index 4faa2781c964..68c1949a253f 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -1510,8 +1510,8 @@  static int drbg_instantiate(struct drbg_state *drbg, struct drbg_string *pers,
 	return ret;
 
 free_everything:
-	mutex_unlock(&drbg->drbg_mutex);
 	drbg_uninstantiate(drbg);
+	mutex_unlock(&drbg->drbg_mutex);
 	return ret;
 }