From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1423712AbdDUSYc (ORCPT ); Fri, 21 Apr 2017 14:24:32 -0400 Received: from mail-io0-f194.google.com ([209.85.223.194]:33401 "EHLO mail-io0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1162350AbdDUSYW (ORCPT ); Fri, 21 Apr 2017 14:24:22 -0400 Date: Fri, 21 Apr 2017 11:24:18 -0700 From: Eric Biggers To: David Howells Cc: keyrings@vger.kernel.org, linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org, Eric Biggers , Mimi Zohar , David Safford Subject: Re: [PATCH 3/5] KEYS: encrypted: sanitize all key material Message-ID: <20170421182418.GA12755@gmail.com> References: <20170421083037.12746-4-ebiggers3@gmail.com> <20170421083037.12746-1-ebiggers3@gmail.com> <28373.1492785068@warthog.procyon.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <28373.1492785068@warthog.procyon.org.uk> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Apr 21, 2017 at 03:31:08PM +0100, David Howells wrote: > Eric Biggers wrote: > > > - memzero_explicit(epayload->decrypted_data, epayload->decrypted_datalen); > > - kfree(key->payload.data[0]); > > + kzfree(key->payload.data[0]); > > Should kzfree() be using memzero_explicit() rather than memset()? > > David It's not actually needed because it's impossible for the compiler to optimize away the memset(). memzero_explicit() is only needed on stack data. The reason I still used memzero_explicit() for heap data in a couple of my patches, even though it's unnecessary, is just that it makes it clearer that it's being done for sanitization purposes, as opposed to some random memset. That's not as much of an issue for kzfree(), since it's explicitly for sanitization purposes already. As a separate note, something that might make sense at some point would be to skip the memset in kzfree() if slab poisoning is enabled. Eric