Messages in this thread Patch in this message | | | Subject | Re: [PATCH v6 2/7] fpga: sec-mgr: enable secure updates | From | Martin Hundebøll <> | Date | Tue, 1 Dec 2020 09:47:20 +0100 |
| |
Hi Russ,
On 01/12/2020 00.54, Russ Weight wrote: > Thanks Martin. I'll work on a fix for this.
Attached is my in-house fix.
// Martin
> On 11/26/20 6:02 AM, Martin Hundebøll wrote: >> Hi Russ, >> >> I found another thing while testing this... >> >> On 06/11/2020 02.09, Russ Weight wrote: >> >> <snip> >> >>> +static ssize_t filename_store(struct device *dev, struct device_attribute *attr, >>> + const char *buf, size_t count) >>> +{ >>> + struct fpga_sec_mgr *smgr = to_sec_mgr(dev); >>> + int ret = count; >>> + >>> + if (count == 0 || count >= PATH_MAX) >>> + return -EINVAL; >>> + >>> + mutex_lock(&smgr->lock); >>> + if (smgr->driver_unload || smgr->progress != FPGA_SEC_PROG_IDLE) { >>> + ret = -EBUSY; >>> + goto unlock_exit; >>> + } >>> + >>> + smgr->filename = kstrndup(buf, count - 1, GFP_KERNEL); >> >> The `count - 1` is meant to remove a trailing newline, but opae-sdk writes the filename without newline, so better do it conditionally... >> >>> + if (!smgr->filename) { >>> + ret = -ENOMEM; >>> + goto unlock_exit; >>> + } >>> + >>> + smgr->err_code = FPGA_SEC_ERR_NONE; >>> + smgr->progress = FPGA_SEC_PROG_READING; >>> + reinit_completion(&smgr->update_done); >>> + schedule_work(&smgr->work); >>> + >>> +unlock_exit: >>> + mutex_unlock(&smgr->lock); >>> + return ret; >>> +} >>> +static DEVICE_ATTR_WO(filename); >>> + >>> +static struct attribute *sec_mgr_update_attrs[] = { >>> + &dev_attr_filename.attr, >>> + NULL, >>> +}; >> >> Thanks, >> Martin > From 23113231c7819b2e99854cac1ec4370b06db442c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Hundeb=C3=B8ll?= <mhu@silicom.dk> Date: Thu, 26 Nov 2020 14:19:51 +0100 Subject: [PATCH] fpga: fpga-sec-mgr: handle trailing newline in filename_store MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
The copying of the filename written to the sysfs unconditionally remove the last character, as (I assume) if it were a newline char. For the most cases this is true (e.g. when using `echo` in the shell), but some software writes the filename without a trailing newline, in which the firmware load obviously fails.
Fix this by checking for a newline char, and replacing it with '\0' if found.
Fixes: 1815cc58d473 ("fpga: sec-mgr: enable secure updates") Signed-off-by: Martin Hundebøll <mhu@silicom.dk> --- drivers/fpga/fpga-sec-mgr.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/fpga/fpga-sec-mgr.c b/drivers/fpga/fpga-sec-mgr.c index 72b61dc173db..d2ce4c682bd9 100644 --- a/drivers/fpga/fpga-sec-mgr.c +++ b/drivers/fpga/fpga-sec-mgr.c @@ -423,12 +423,16 @@ static ssize_t filename_store(struct device *dev, struct device_attribute *attr, goto unlock_exit; } - smgr->filename = kstrndup(buf, count - 1, GFP_KERNEL); + smgr->filename = kstrndup(buf, count, GFP_KERNEL); if (!smgr->filename) { ret = -ENOMEM; goto unlock_exit; } + /* remove trailing newline */ + if (smgr->filename[count - 1] == '\n') + smgr->filename[count - 1] = '\0'; + smgr->err_code = FPGA_SEC_ERR_NONE; smgr->hw_errinfo = 0; smgr->request_cancel = false; -- 2.29.2
| |