From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752669AbcEKMmu (ORCPT ); Wed, 11 May 2016 08:42:50 -0400 Received: from e23smtp04.au.ibm.com ([202.81.31.146]:43357 "EHLO e23smtp04.au.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752001AbcEKMms (ORCPT ); Wed, 11 May 2016 08:42:48 -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: <1462970512.12106.91.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 08:41:52 -0400 In-Reply-To: <1462919163-2503-4-git-send-email-stephen.boyd@linaro.org> References: <1462919163-2503-1-git-send-email-stephen.boyd@linaro.org> <1462919163-2503-4-git-send-email-stephen.boyd@linaro.org> 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: 16051112-0013-0000-0000-0000051E5FE3 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 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? There has to be a more appropriate string identifier. > + msize = buf->allocated_size; > + } > > path = __getname(); > if (!path) > --- a/fs/exec.c > +++ b/fs/exec.c > @@ -836,8 +836,8 @@ int kernel_read(struct file *file, loff_t offset, > > EXPORT_SYMBOL(kernel_read); > > -int kernel_read_file(struct file *file, void **buf, loff_t *size, > - loff_t max_size, enum kernel_read_file_id id) > +static int _kernel_read_file(struct file *file, void **buf, loff_t *size, > + loff_t max_size, enum kernel_read_file_id id) > { > loff_t i_size, pos; > ssize_t bytes = 0; > @@ -856,7 +856,8 @@ int kernel_read_file(struct file *file, void **buf, loff_t *size, > if (i_size <= 0) > return -EINVAL; > > - *buf = vmalloc(i_size); > + if (id != READING_FIRMWARE_INTO_BUF) > + *buf = vmalloc(i_size); > if (!*buf) > return -ENOMEM; > > @@ -885,11 +886,19 @@ int kernel_read_file(struct file *file, void **buf, loff_t *size, > > out: > if (ret < 0) { > - vfree(*buf); > - *buf = NULL; > + if (id != READING_FIRMWARE_INTO_BUF) { > + vfree(*buf); > + *buf = NULL; > + } > } > return ret; > } > + > +int kernel_read_file(struct file *file, void **buf, loff_t *size, > + loff_t max_size, enum kernel_read_file_id id) > +{ > + return _kernel_read_file(file, buf, size, max_size, id); > +} > EXPORT_SYMBOL_GPL(kernel_read_file); This seems to be exactly the same as the original version. > > int kernel_read_file_from_path(char *path, void **buf, loff_t *size, > @@ -905,7 +914,7 @@ int kernel_read_file_from_path(char *path, void **buf, loff_t *size, > if (IS_ERR(file)) > return PTR_ERR(file); > > - ret = kernel_read_file(file, buf, size, max_size, id); > + ret = _kernel_read_file(file, buf, size, max_size, id); > fput(file); > return ret; > } > diff --git a/include/linux/firmware.h b/include/linux/firmware.h > index 5c41c5e75b5c..bdc24ee92823 100644 > --- a/include/linux/firmware.h > +++ b/include/linux/firmware.h > @@ -47,6 +47,8 @@ int request_firmware_nowait( > void (*cont)(const struct firmware *fw, void *context)); > int request_firmware_direct(const struct firmware **fw, const char *name, > struct device *device); > +int request_firmware_into_buf(const struct firmware **firmware_p, > + const char *name, struct device *device, void *buf, size_t size); > > void release_firmware(const struct firmware *fw); > #else > @@ -75,5 +77,11 @@ static inline int request_firmware_direct(const struct firmware **fw, > return -EINVAL; > } > > +static inline int request_firmware_into_buf(const struct firmware **firmware_p, > + const char *name, struct device *device, void *buf, size_t size); > +{ > + return -EINVAL; > +} > + > #endif > #endif > diff --git a/include/linux/fs.h b/include/linux/fs.h > index 14a97194b34b..4fb8596b3dd4 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -2582,6 +2582,7 @@ extern int do_pipe_flags(int *, int); > > enum kernel_read_file_id { > READING_FIRMWARE = 1, > + READING_FIRMWARE_INTO_BUF, > READING_MODULE, > READING_KEXEC_IMAGE, > READING_KEXEC_INITRAMFS, Commit "1284ab5 fs: define a string representation of the kernel_read_file_id enumeration", which is queued to be upstreamed, changes this a bit. > diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c > index 60d011aaec38..b922d6ca2fb0 100644 > --- a/security/integrity/ima/ima_fs.c > +++ b/security/integrity/ima/ima_fs.c > @@ -272,7 +272,8 @@ static ssize_t ima_read_policy(char *path) > datap = path; > strsep(&datap, "\n"); > > - rc = kernel_read_file_from_path(path, &data, &size, 0, READING_POLICY); > + rc = kernel_read_file_from_path(path, &data, &size, 0, READING_POLICY, > + NULL); It doesn't look like kernel_read_file_from_path() changed. Mimi > if (rc < 0) { > pr_err("Unable to open file: %s (%d)", path, rc); > return rc;