diff mbox

mtd: mtdram: check offs and len in mtdram->erase

Message ID 1443631303-22057-1-git-send-email-sudipm.mukherjee@gmail.com
State Superseded
Headers show

Commit Message

Sudip Mukherjee Sept. 30, 2015, 4:41 p.m. UTC
We should prevent user to erasing mtd device with an unaligned offset
or length.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---

I am not sure if I should add the Signed-off-by of
Dongsheng Yang <yangds.fnst@cn.fujitsu.com> . He is the original author
and he should get the credit for that.

 drivers/mtd/devices/mtdram.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

Comments

Dongsheng Yang Oct. 2, 2015, 9:39 a.m. UTC | #1
On 10/01/2015 12:41 AM, Sudip Mukherjee wrote:
> We should prevent user to erasing mtd device with an unaligned offset
> or length.
>
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> ---
>
> I am not sure if I should add the Signed-off-by of
> Dongsheng Yang <yangds.fnst@cn.fujitsu.com> . He is the original author
> and he should get the credit for that.

But I had sent a a patch out to fix this problem before your v1.

http://lists.infradead.org/pipermail/linux-mtd/2015-September/062234.html

Yang
>
>   drivers/mtd/devices/mtdram.c | 27 +++++++++++++++++++++++++++
>   1 file changed, 27 insertions(+)
>
> diff --git a/drivers/mtd/devices/mtdram.c b/drivers/mtd/devices/mtdram.c
> index 8e28508..21b6a05 100644
> --- a/drivers/mtd/devices/mtdram.c
> +++ b/drivers/mtd/devices/mtdram.c
> @@ -32,8 +32,35 @@ MODULE_PARM_DESC(erase_size, "Device erase block size in KiB");
>   // We could store these in the mtd structure, but we only support 1 device..
>   static struct mtd_info *mtd_info;
>
> +static int check_offs_len(struct mtd_info *mtd, loff_t ofs, uint64_t len)
> +{
> +	int ret = 0;
> +	uint64_t temp_len, rem;
> +
> +	/* Start address must align on block boundary */
> +	temp_len = ofs;
> +	rem = do_div(temp_len, mtd->erasesize);
> +	if (rem) {
> +		pr_debug("%s: unaligned address\n", __func__);
> +		ret = -EINVAL;
> +	}
> +
> +	/* Length must align on block boundary */
> +	temp_len = len;
> +	rem = do_div(temp_len, mtd->erasesize);
> +
> +	if (rem) {
> +		pr_debug("%s: length not block aligned\n", __func__);
> +		ret = -EINVAL;
> +	}
> +
> +	return ret;
> +}
> +
>   static int ram_erase(struct mtd_info *mtd, struct erase_info *instr)
>   {
> +	if (check_offs_len(mtd, instr->addr, instr->len))
> +		return -EINVAL;
>   	memset((char *)mtd->priv + instr->addr, 0xff, instr->len);
>   	instr->state = MTD_ERASE_DONE;
>   	mtd_erase_callback(instr);
>
Sudip Mukherjee Oct. 2, 2015, 10:01 a.m. UTC | #2
On Fri, Oct 02, 2015 at 05:39:02PM +0800, Dongsheng Yang wrote:
> On 10/01/2015 12:41 AM, Sudip Mukherjee wrote:
> >We should prevent user to erasing mtd device with an unaligned offset
> >or length.
> >
> >Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> >---
> >
> >I am not sure if I should add the Signed-off-by of
> >Dongsheng Yang <yangds.fnst@cn.fujitsu.com> . He is the original author
> >and he should get the credit for that.
> 
> But I had sent a a patch out to fix this problem before your v1.
> 
> http://lists.infradead.org/pipermail/linux-mtd/2015-September/062234.html
I didn't know that. I think your v1 was applied.

regards
sudip
Brian Norris Oct. 2, 2015, 5:38 p.m. UTC | #3
On Fri, Oct 02, 2015 at 03:31:33PM +0530, Sudip Mukherjee wrote:
> On Fri, Oct 02, 2015 at 05:39:02PM +0800, Dongsheng Yang wrote:
> > On 10/01/2015 12:41 AM, Sudip Mukherjee wrote:
> > >We should prevent user to erasing mtd device with an unaligned offset
> > >or length.
> > >
> > >Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> > >---
> > >
> > >I am not sure if I should add the Signed-off-by of
> > >Dongsheng Yang <yangds.fnst@cn.fujitsu.com> . He is the original author
> > >and he should get the credit for that.
> > 
> > But I had sent a a patch out to fix this problem before your v1.
> > 
> > http://lists.infradead.org/pipermail/linux-mtd/2015-September/062234.html
> I didn't know that. I think your v1 was applied.

Sorry if I left any confusion. Dongsheng's v1 was applied and reverted.
v2 is still under review (and was sent slightly before (?) your v1).
Feel free to comment there.

Brian
Dongsheng Yang Oct. 3, 2015, 3:31 a.m. UTC | #4
On 10/03/2015 01:38 AM, Brian Norris wrote:
> On Fri, Oct 02, 2015 at 03:31:33PM +0530, Sudip Mukherjee wrote:
>> On Fri, Oct 02, 2015 at 05:39:02PM +0800, Dongsheng Yang wrote:
>>> On 10/01/2015 12:41 AM, Sudip Mukherjee wrote:
>>>> We should prevent user to erasing mtd device with an unaligned offset
>>>> or length.
>>>>
>>>> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
>>>> ---
>>>>
>>>> I am not sure if I should add the Signed-off-by of
>>>> Dongsheng Yang <yangds.fnst@cn.fujitsu.com> . He is the original author
>>>> and he should get the credit for that.
>>>
>>> But I had sent a a patch out to fix this problem before your v1.
>>>
>>> http://lists.infradead.org/pipermail/linux-mtd/2015-September/062234.html
>> I didn't know that. I think your v1 was applied.
>
> Sorry if I left any confusion. Dongsheng's v1 was applied and reverted.
> v2 is still under review (and was sent slightly before (?) your v1).

Yea, sorry I should have mentioned it earlier. But I was and am still
in a vacation, then I did not point it out in time.

Sudip, any comment or test for my patch there is always welcome.

Thanx
Yang
> Feel free to comment there.
>
> Brian
> .
>
Sudip Mukherjee Oct. 3, 2015, 5:42 a.m. UTC | #5
On Sat, Oct 03, 2015 at 11:31:04AM +0800, Dongsheng Yang wrote:
> On 10/03/2015 01:38 AM, Brian Norris wrote:
> >On Fri, Oct 02, 2015 at 03:31:33PM +0530, Sudip Mukherjee wrote:
> >>On Fri, Oct 02, 2015 at 05:39:02PM +0800, Dongsheng Yang wrote:
> >>>On 10/01/2015 12:41 AM, Sudip Mukherjee wrote:
> >>>>We should prevent user to erasing mtd device with an unaligned offset
> >>>>or length.
> >>>>
> >>>>Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> >>>>---
> >>>>
> >>>>I am not sure if I should add the Signed-off-by of
> >>>>Dongsheng Yang <yangds.fnst@cn.fujitsu.com> . He is the original author
> >>>>and he should get the credit for that.
> >>>
> >>>But I had sent a a patch out to fix this problem before your v1.
> >>>
> >>>http://lists.infradead.org/pipermail/linux-mtd/2015-September/062234.html
> >>I didn't know that. I think your v1 was applied.
> >
> >Sorry if I left any confusion. Dongsheng's v1 was applied and reverted.
> >v2 is still under review (and was sent slightly before (?) your v1).
> 
> Yea, sorry I should have mentioned it earlier. But I was and am still
> in a vacation, then I did not point it out in time.
> 
> Sudip, any comment or test for my patch there is always welcome.
Sorry, I donot know anything about this driver to comment. My main patch
was to fix the build failure. And since by that time your patch was
reverted so I sent another path with my patch and your patch combined
together.

regards
sudip
diff mbox

Patch

diff --git a/drivers/mtd/devices/mtdram.c b/drivers/mtd/devices/mtdram.c
index 8e28508..21b6a05 100644
--- a/drivers/mtd/devices/mtdram.c
+++ b/drivers/mtd/devices/mtdram.c
@@ -32,8 +32,35 @@  MODULE_PARM_DESC(erase_size, "Device erase block size in KiB");
 // We could store these in the mtd structure, but we only support 1 device..
 static struct mtd_info *mtd_info;
 
+static int check_offs_len(struct mtd_info *mtd, loff_t ofs, uint64_t len)
+{
+	int ret = 0;
+	uint64_t temp_len, rem;
+
+	/* Start address must align on block boundary */
+	temp_len = ofs;
+	rem = do_div(temp_len, mtd->erasesize);
+	if (rem) {
+		pr_debug("%s: unaligned address\n", __func__);
+		ret = -EINVAL;
+	}
+
+	/* Length must align on block boundary */
+	temp_len = len;
+	rem = do_div(temp_len, mtd->erasesize);
+
+	if (rem) {
+		pr_debug("%s: length not block aligned\n", __func__);
+		ret = -EINVAL;
+	}
+
+	return ret;
+}
+
 static int ram_erase(struct mtd_info *mtd, struct erase_info *instr)
 {
+	if (check_offs_len(mtd, instr->addr, instr->len))
+		return -EINVAL;
 	memset((char *)mtd->priv + instr->addr, 0xff, instr->len);
 	instr->state = MTD_ERASE_DONE;
 	mtd_erase_callback(instr);