Differential D6442
Make more use of arc4random() in the kernel. Authored by pfg on May 18 2016, 8:21 PM. Tags None Referenced Files
Details This is an attempt to use arc4random() more extensively in the kernel. I excluded using arc4random in contrib'ed code, in tests, in devices and in the screensavers. devices may have to be investigated later. I rebuilt my kernel and I am running it.
Diff Detail
Event TimelineComment Actions Hello; The improvement is probably not huge but given that we can provide better randomness ... why not? :) This touches a bunch of code in the kernel and I want to make sure the code was not side-stepped in the past from conversion for some reason. Thanks for reviewing. Comment Actions Without numbers quantifying the impact of switching to arc4random(), it is difficult to agree this is a good idea or provide any useful feedback. I don't possess any intuitive notion of the overhead vs entropy improvement compared with random(). Comment Actions From my measurements in filesystems, the performance difference was not quantifiable. We just happen to use random numbers very rarely in filesystems and it basically happens during formatting.
It very much depends on the application and the type of load so I am not suggesting just committing this patch. It has to be evaluated by different developers and it's unlikely we could do this for, say, 11-Release. Perhaps we could incorporate it early in the 12-current cycle though which would give it ample testing for a year or two. For now I would appreciate reviews of the application: there was some scaling that had to be adjusted for the congestion algorithms. Are those OK? Comment Actions I think we should get Chacha20, getentropy(), etc. first. Please do not use arc4random() % but instead use arc4random_uniform (we should probably get ChaCha20 in base first) by the way.
Comment Actions I think that is a PR with a patch for that, but an important part
We don't have arc4random_uniform in the kernel. I am considering dropping completely this patch and instead rewrite random() to use a better and faster, but not crypto-safe, algorithm based on KISS. Comment Actions Add cem@.
Comment Actions I think it would be more straightforward to divide this into reviews by subject code area. Feel free to cc me in all of those if you do so. I’m happier to see more use of arc4random when it’s a good fit. As far as earlier concerns: arc4random(9) and (3) are now chacha-based in 12 and newer; but we still lack arc4random_uniform() in CURRENT. Comment Actions I may still split it. but its not so big anymore :). There were some cases where it was actually wrong to use due to the return type difference.
TBH, arc4random_uniform() is not too important: it basically applies to the cases where one uses the modulus operation to limit the range, so if we do bring it the replacement could be done automatically with coccinelle.
Comment Actions Hmm ... I have separated the interesting, networking-related, changes in different reviews. Most of the remaining changes are just replacing random in conjunction with modulus operations to limit the range, and are not of high value. I'll probably close the review.
Comment Actions Well, the idea is not purely unifying the generators, but to use the same CSPRNG in as much as possible places in kernel to make it harder to predicate the next output, by using these arc4random() calls as another form of randomness introduced to the entropy source (as such, to effectively use the feature the calls has to obtain small quantities of outputs from arc4random()). That's said, I believe we still want to use arc4random() for, at least, the code paths that are not performance critical and do performance analysis and decide carefully if they should be used in the critical ones. Comment Actions Drop revision: none of this is security critical and replacing good old random for the sake of removing it is not an objective. Comment Actions Theo de Raadt was at some point argumenting that arc4random was extremely fast: he claimed the same perfomance as memset ( but I wouldn't trust such claim). In all the cases above, the changes have no performance implications simply because we use random numbers very rarely and punctually so I doubt we would notice any difference. Ultimately, and speaking of the above cases, performance doesn't matter. What disappoints me, and made me drop the change, is that the quality of randomness in the above cases is of very little relevance. The great advantage of arc4random(), as I see it, is doubling the range of randomness When you use the modulus operation to restrict the range you lose such "randomness granularity" and there is no advantage in using arc4random. Furthermore, the value itself has no consequence security-wise. Having a random:_uniform function is also of little value in these cases, we should probably not waste out time adding such interface. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||