| Message ID | 4cb7a3dd-9eac-41c5-8498-f2695f01576c@mary.at.omicron.at |
|---|---|
| State | Accepted |
| Commit | 9a78bc83b4c31f67202b7b0a77fa25da732f44a3 |
| Headers | show |
On Thu, Mar 06, 2014 at 12:42:37PM +0100, Christian Riesch wrote: > If a write to one time programmable memory (OTP) hits the end of this > memory area, no more data can be written. The count variable in > mtdchar_write() in drivers/mtd/mtdchar.c is not decreased anymore. > We are trapped in the loop forever, mtdchar_write() will never return > in this case. > > The desired behavior of a write in such a case is described in [1]: > - Try to write as much data as possible, truncate the write to fit into > the available memory and return the number of bytes that actually > have been written. > - If no data could be written at all, return -ENOSPC. > > This patch fixes the behavior of OTP write if there is not enough space > for all data: > > 1) mtd_write_user_prot_reg() in drivers/mtd/mtdcore.c is modified to > return -ENOSPC if no data could be written at all. > 2) mtdchar_write() is modified to handle -ENOSPC correctly. Exit if a > write returned -ENOSPC and yield the correct return value, either > then number of bytes that could be written, or -ENOSPC, if no data > could be written at all. > > Furthermore the patch harmonizes the behavior of the OTP memory write > in drivers/mtd/devices/mtd_dataflash.c with the other implementations > and the requirements from [1]. Instead of returning -EINVAL if the data > does not fit into the OTP memory, we try to write as much data as > possible/truncate the write. > > [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/write.html > > Signed-off-by: Christian Riesch <christian.riesch@omicron.at> > Cc: Brian Norris <computersforpeace@gmail.com> > --- > > Changes for v3: > - Moved logic for returning -ENOSPC from cfi_intelext_write_user_prot_reg, > dataflash_write_user_otp, onenand_write_user_prot_reg to a common > implementation in mtd_write_user_prot_reg, as requested by Brian Norris. > - Simplified the code in mtdchar_write as suggested by Brian. > - Added more comments, commit message. Thanks for the update. Pushed to l2-mtd.git! Brian
--On March 08, 2014 20:56 -0800 Brian Norris <computersforpeace@gmail.com> wrote: > On Thu, Mar 06, 2014 at 12:42:37PM +0100, Christian Riesch wrote: >> If a write to one time programmable memory (OTP) hits the end of this >> memory area, no more data can be written. The count variable in >> mtdchar_write() in drivers/mtd/mtdchar.c is not decreased anymore. >> We are trapped in the loop forever, mtdchar_write() will never return >> in this case. [...] >> Signed-off-by: Christian Riesch <christian.riesch@omicron.at> >> Cc: Brian Norris <computersforpeace@gmail.com> >> --- [...] > Thanks for the update. Pushed to l2-mtd.git! Thanks! Based on this work I have now also reworked my patches for OTP support for drivers/mtd/chips/cfi_cmdset_0002.c. The discussion on earlier versions of this patchset led to the two fixes that you took into l2-mtd now. I am looking forward to your comments on v3 of the patchset [1]. Christian [1] http://lists.infradead.org/pipermail/linux-mtd/2014-March/052422.html
diff --git a/drivers/mtd/devices/mtd_dataflash.c b/drivers/mtd/devices/mtd_dataflash.c index c89f1c5..10b7608 100644 --- a/drivers/mtd/devices/mtd_dataflash.c +++ b/drivers/mtd/devices/mtd_dataflash.c @@ -543,14 +543,18 @@ static int dataflash_write_user_otp(struct mtd_info *mtd, struct dataflash *priv = mtd->priv; int status; - if (len > 64) - return -EINVAL; + if (from >= 64) { + /* + * Attempting to write beyond the end of OTP memory, + * no data can be written. + */ + *retlen = 0; + return 0; + } - /* Strictly speaking, we *could* truncate the write ... but - * let's not do that for the only write that's ever possible. - */ + /* Truncate the write to fit into OTP memory. */ if ((from + len) > 64) - return -EINVAL; + len = 64 - from; /* OUT: OP_WRITE_SECURITY, 3 zeroes, 64 data-or-zero bytes * IN: ignore all diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c index 250798c..7d4e7b9 100644 --- a/drivers/mtd/mtdchar.c +++ b/drivers/mtd/mtdchar.c @@ -324,6 +324,15 @@ static ssize_t mtdchar_write(struct file *file, const char __user *buf, size_t c default: ret = mtd_write(mtd, *ppos, len, &retlen, kbuf); } + + /* + * Return -ENOSPC only if no data could be written at all. + * Otherwise just return the number of bytes that actually + * have been written. + */ + if ((ret == -ENOSPC) && (total_retlen)) + break; + if (!ret) { *ppos += retlen; total_retlen += retlen; diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c index 0a7d77e..d201fee 100644 --- a/drivers/mtd/mtdcore.c +++ b/drivers/mtd/mtdcore.c @@ -932,12 +932,22 @@ EXPORT_SYMBOL_GPL(mtd_read_user_prot_reg); int mtd_write_user_prot_reg(struct mtd_info *mtd, loff_t to, size_t len, size_t *retlen, u_char *buf) { + int ret; + *retlen = 0; if (!mtd->_write_user_prot_reg) return -EOPNOTSUPP; if (!len) return 0; - return mtd->_write_user_prot_reg(mtd, to, len, retlen, buf); + ret = mtd->_write_user_prot_reg(mtd, to, len, retlen, buf); + if (ret) + return ret; + + /* + * If no data could be written at all, we are out of memory and + * must return -ENOSPC. + */ + return (*retlen) ? 0 : -ENOSPC; } EXPORT_SYMBOL_GPL(mtd_write_user_prot_reg);
If a write to one time programmable memory (OTP) hits the end of this memory area, no more data can be written. The count variable in mtdchar_write() in drivers/mtd/mtdchar.c is not decreased anymore. We are trapped in the loop forever, mtdchar_write() will never return in this case. The desired behavior of a write in such a case is described in [1]: - Try to write as much data as possible, truncate the write to fit into the available memory and return the number of bytes that actually have been written. - If no data could be written at all, return -ENOSPC. This patch fixes the behavior of OTP write if there is not enough space for all data: 1) mtd_write_user_prot_reg() in drivers/mtd/mtdcore.c is modified to return -ENOSPC if no data could be written at all. 2) mtdchar_write() is modified to handle -ENOSPC correctly. Exit if a write returned -ENOSPC and yield the correct return value, either then number of bytes that could be written, or -ENOSPC, if no data could be written at all. Furthermore the patch harmonizes the behavior of the OTP memory write in drivers/mtd/devices/mtd_dataflash.c with the other implementations and the requirements from [1]. Instead of returning -EINVAL if the data does not fit into the OTP memory, we try to write as much data as possible/truncate the write. [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/write.html Signed-off-by: Christian Riesch <christian.riesch@omicron.at> Cc: Brian Norris <computersforpeace@gmail.com> --- Changes for v3: - Moved logic for returning -ENOSPC from cfi_intelext_write_user_prot_reg, dataflash_write_user_otp, onenand_write_user_prot_reg to a common implementation in mtd_write_user_prot_reg, as requested by Brian Norris. - Simplified the code in mtdchar_write as suggested by Brian. - Added more comments, commit message. drivers/mtd/devices/mtd_dataflash.c | 16 ++++++++++------ drivers/mtd/mtdchar.c | 9 +++++++++ drivers/mtd/mtdcore.c | 12 +++++++++++- 3 files changed, 30 insertions(+), 7 deletions(-)