From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752386AbcEKUrq (ORCPT ); Wed, 11 May 2016 16:47:46 -0400 Received: from e23smtp02.au.ibm.com ([202.81.31.144]:54527 "EHLO e23smtp02.au.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751462AbcEKUrp (ORCPT ); Wed, 11 May 2016 16:47:45 -0400 X-IBM-Helo: d23dlp02.au.ibm.com X-IBM-MailFrom: zohar@linux.vnet.ibm.com X-IBM-RcptTo: linux-kernel@vger.kernel.org Message-ID: <1462999606.12416.37.camel@linux.vnet.ibm.com> Subject: Re: [RFC/PATCHv3 v3 3/3] firmware: Support loading into a pre-allocated buffer From: Mimi Zohar To: Stephen Boyd Cc: linux-kernel@vger.kernel.org, linux-arm@lists.infradead.org, Andrew Morton , Mark Brown , Ming Lei , Vikram Mulukutla Date: Wed, 11 May 2016 16:46:46 -0400 In-Reply-To: <20160511202231.8857.86068@sboyd-linaro> References: <1462919163-2503-1-git-send-email-stephen.boyd@linaro.org> <1462919163-2503-4-git-send-email-stephen.boyd@linaro.org> <1462970512.12106.91.camel@linux.vnet.ibm.com> <20160511202231.8857.86068@sboyd-linaro> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.12.11 (3.12.11-1.fc21) Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16051120-0005-0000-0000-000007B84E20 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2016-05-11 at 13:22 -0700, Stephen Boyd wrote: > Quoting Mimi Zohar (2016-05-11 05:41:52) > > Hi Stephen, > > > > On Tue, 2016-05-10 at 15:26 -0700, Stephen Boyd wrote: > > > > > -static int fw_get_filesystem_firmware(struct device *device, > > > - struct firmware_buf *buf) > > > +static int > > > +fw_get_filesystem_firmware(struct device *device, struct firmware_buf *buf) > > > { > > > loff_t size; > > > int i, len; > > > int rc = -ENOENT; > > > char *path; > > > + enum kernel_read_file_id id = READING_FIRMWARE; > > > + size_t msize = INT_MAX; > > > + > > > + /* Already populated data member means we're loading into a buffer */ > > > + if (buf->data) { > > > + id = READING_FIRMWARE_INTO_BUF; > > > > In both cases we're reading the firmware into a buffer. In this case, > > it is pre-allocated. Other than it being pre-allocated, is there > > anything special about this buffer? > > No. I'm not sure what you're asking/implying. > > > There has to be a more appropriate > > string identifier. > > Ok. Any suggestions? The point of the new id is so that we know if we > need to allocate the buffer or not in kernel_read_file(). If you're still using DMA, then perhaps "READING_FIRMWARE_DMA". If the only difference is that the buffer is pre-allocated, then maybe "READING_FIRMWARE_PREALLOC_BUFFER". > Alternatively > we could indicate that by a NULL *buf pointer, but it seems that half > the time that's not initialized to NULL so it didn't seem safe to rely > on that fact or update the callsites appropriately. Assuming that the pre-allocated buffer is smaller than the firmware size, then a new name definitely needs to be specified, so that the firmware signature can be verified on the security_kernel_read_file() hook, as opposed to the security_kernel_post_read_file() hook. The patch would look something like: diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 68b26c3..c799459 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -359,6 +359,10 @@ int ima_read_file(struct file *file, enum kernel_read_file_id read_id) #endif return 0; /* We rely on module signature checking */ } + + if (file && read_id == READING_FIRMWARE_INTO_BUF) + return process_measurement(file, NULL, 0, MAY_READ, + FIRMWARE_CHECK, 0); return 0; } @@ -404,6 +408,9 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size, return 0; } + if (file && read_id == READING_FIRMWARE_INTO_BUF) + return 0; + func = read_idmap[read_id] ?: FILE_CHECK; return process_measurement(file, buf, size, MAY_READ, func, 0); } Once we've decided on a more appropriate identifier string, I'll post the patch. Mimi