On Mon, Feb 01, 2021 at 13:52:11 +0000, Daniel Berrange wrote: > On Mon, Feb 01, 2021 at 02:38:52PM +0100, Peter Krempa wrote: > > Most callers are way better off using memset directly additionally few > > places didn't even use it to clear sensitive data in the first place > > since the name probably sounded as the right thing to use. > > Although virDispose did indeed use memset(), I don't think we should > be replacing it with use of memset(). This is well known to be subject > to compiler optimization eliminating the call entirely. > > We shouldn't have used it in virDispose in the first place, instead > we need to call the platform specific "safe" method for erasing > data. Istead we ought to have been using explicit_bzero or > memset_s(), or memset_explicitly, or $whatever. > > At least with virDispose we would only have one place to fix this > problem, but this with series eliminating it, the callers that need > the secure erase are no longer distinct/visible from general memset > usage. > > I think we ought to have a 'virSecureErase' function, that we can > back with the appropriate platform specific call. > > If you don't want to get so deeply involved in that, I'd be > fine if this series too a minimialist approach and only introduced > > #define virSecureErase(ptr, len) memset(ptr, 0, len) > > and then used virSecureErase intead of memset(). That would at least > make sure we're no worse than today and callers remain easily > identifiable. I will do that but it's worth mentioning that it might give us a false sense of security since there's a lot of memset usage (prior to this series, one place is visible in 11/14, where the encryption key used with gnutls is cleared memset) thus any series wanting to do something else than memset in virSecureErase will need to go through all of memset calls anyways.
