On 02/09/2020 18:08, Dave Martin wrote: > On Mon, Aug 31, 2020 at 12:04:49PM +0100, Boyan Karatotev wrote: >> +/* >> + * fork() does not change keys. Only exec() does so call a worker program. >> + * Its only job is to sign a value and report back the resutls >> + */ >> +TEST(exec_unique_keys) >> +{ > > The kernel doesn't guarantee that keys are unique. > > Can we present all the "unique keys" wording differently, say > > exec_key_collision_likely() I agree that this test's name is a bit out of place. I would rather have it named "exec_changed_keys" though. > Otherwise people might infer from this test code that the keys are > supposed to be truly unique and start reporting bugs on the kernel. > > I can't see an obvious security argument for unique keys (rather, the > keys just need to be "unique enough". That's the job of > get_random_bytes().) The "exec_unique_keys" test only checks that the keys changed after an exec() which I think the name change would reflect. The thing with the "single_thread_unique_keys" test is that the kernel says the the keys will be random. Yes, there is no uniqueness guarantee but I'm not sure how to phrase it differently. There is some minuscule chance that the keys end up the same, but for this test I pretend this will not happen. Would changing up the comments and the failure message communicate this? Maybe substitute "unique" for "different" and say how many keys clashed? -- Regards, Boyan