[LTP] [PATCH 2/2] Add CVE-2017-18075, pcrypt mishandles freeing instances
Cyril Hrubis
chrubis@suse.cz
Thu Mar 15 12:44:56 CET 2018
Hi!
> > runtest/cve | 1 +
> > testcases/cve/.gitignore | 1 +
> > testcases/cve/cve-2017-18075.c | 201 +++++++++++++++++++++++++++++++++++++++++
>
> Thanks for writing an LTP test for this!
>
> Just my 2 cents, but I think it is insane to be naming tests after CVE numbers
> instead of putting them in an appropriate place, like a crypto/ directory for
> this one. People aren't going to remember what "CVE-2017-18075" is. I'm even
> the person who fixed this bug and requested this CVE, and I still didn't
> recognize the CVE number; this patch only drew my attention because the subject
> line mentioned pcrypt. (And now I see that I missed the recent test for the
> modify_ldt() use-after-free bug because the patch subject line and description
> only mentioned "CVE-2017-17053".)
Agreed, the numbers suck, I have to read everything twice to avoid
typos.
Maybe we should name the test files after the kernel subsystem with an
increasing counter as a last resort.
> I suggest putting this NETLINK_CRYPTO stuff in a common location that can be
> used by other tests too. This will not be the last crypto API bug. The
> definitions for AF_ALG probably should be there too; though AF_ALG isn't used by
> this test, many crypto bugs I've fixed or seen fixed recently are accessible
> through it. (E.g. see commit ecaaab564978, "crypto: salsa20 - fix
> blkcipher_walk API usage" or commit e57121d08c38, "crypto: chacha20poly1305 -
> validate the digest size". Sorry, I was a bit lazy by just putting reproducers
> in the commit messages and not writing "real" tests.) It would be great to have
> helper functions in LTP for testing the crypto API, so that they don't have to
> be repeated in every test.
We do have include/lapi/ headers for that purpose, we may as well put it
there.
--
Cyril Hrubis
chrubis@suse.cz
More information about the ltp
mailing list