[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9f96baf1-962e-d595-0e4f-797315cd0348@huawei.com>
Date: Wed, 11 Oct 2023 10:32:45 +0800
From: ZhaoLong Wang <wangzhaolong1@...wei.com>
To: <richard@....at>, <miquel.raynal@...tlin.com>, <vigneshr@...com>
CC: <linux-mtd@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
<chengzhihao1@...wei.com>, <yi.zhang@...wei.com>,
<yangerkun@...wei.com>
Subject: Re: [PATCH RFC] ubi: gluebi: Fix NULL pointer dereference caused by
ftl notifier
> This patch assumes that the gluebi module is not designed to work with
> the ftl module. In this case, the patch only needs to prevent the ftl
> notifier operation.
>
> Add some correctness check for gluebi->desc in gluebi_read/write/erase(),
> If the pointer is invalid, the -EINVAL is returned.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217992 [1]
> Signed-off-by: ZhaoLong Wang <wangzhaolong1@...wei.com>
> ---
> drivers/mtd/ubi/gluebi.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/mtd/ubi/gluebi.c b/drivers/mtd/ubi/gluebi.c
> index 1b980d15d9fb..189ecc0eacd1 100644
> --- a/drivers/mtd/ubi/gluebi.c
> +++ b/drivers/mtd/ubi/gluebi.c
> @@ -157,6 +157,9 @@ static int gluebi_read(struct mtd_info *mtd, loff_t from, size_t len,
> struct gluebi_device *gluebi;
>
> gluebi = container_of(mtd, struct gluebi_device, mtd);
> + if (IS_ERR_OR_NULL(gluebi->desc))
> + return -EINVAL;
> +
> lnum = div_u64_rem(from, mtd->erasesize, &offs);
> bytes_left = len;
> while (bytes_left) {
> @@ -197,6 +200,9 @@ static int gluebi_write(struct mtd_info *mtd, loff_t to, size_t len,
> struct gluebi_device *gluebi;
>
> gluebi = container_of(mtd, struct gluebi_device, mtd);
> + if (IS_ERR_OR_NULL(gluebi->desc))
> + return -EINVAL;
> +
> lnum = div_u64_rem(to, mtd->erasesize, &offs);
>
> if (len % mtd->writesize || offs % mtd->writesize)
> @@ -242,6 +248,8 @@ static int gluebi_erase(struct mtd_info *mtd, struct erase_info *instr)
> lnum = mtd_div_by_eb(instr->addr, mtd);
> count = mtd_div_by_eb(instr->len, mtd);
> gluebi = container_of(mtd, struct gluebi_device, mtd);
> + if (IS_ERR_OR_NULL(gluebi->desc))
> + return -EINVAL;
>
> for (i = 0; i < count - 1; i++) {
> err = ubi_leb_unmap(gluebi->desc, lnum + i);
This modification attempts another solution. Always check the validity
of gluebi->desc. If the gluebi->desc pointer is invalid, try to get MTD
device.
diff --git a/drivers/mtd/ubi/gluebi.c b/drivers/mtd/ubi/gluebi.c
index 1b980d15d9fb..f1a74ccf1718 100644
--- a/drivers/mtd/ubi/gluebi.c
+++ b/drivers/mtd/ubi/gluebi.c
@@ -154,9 +154,19 @@ static int gluebi_read(struct mtd_info *mtd, loff_t
from, size_t len,
size_t *retlen, unsigned char *buf)
{
int err = 0, lnum, offs, bytes_left;
- struct gluebi_device *gluebi;
+ struct gluebi_device *gluebi = container_of(mtd, struct gluebi_device,
+ mtd);
+ int not_get = IS_ERR_OR_NULL(gluebi->desc);
+
+ if (not_get) {
+ err = __get_mtd_device(mtd);
+ if (err) {
+ err_msg("cannot get MTD device %d, UBI device %d, volume %d, error %d",
+ mtd->index, gluebi->ubi_num, gluebi->vol_id, err);
+ return err;
+ }
+ }
- gluebi = container_of(mtd, struct gluebi_device, mtd);
lnum = div_u64_rem(from, mtd->erasesize, &offs);
bytes_left = len;
while (bytes_left) {
@@ -176,6 +186,9 @@ static int gluebi_read(struct mtd_info *mtd, loff_t
from, size_t len,
}
*retlen = len - bytes_left;
+
+ if (not_get)
+ __put_mtd_device(mtd);
return err;
}
@@ -194,9 +207,19 @@ static int gluebi_write(struct mtd_info *mtd,
loff_t to, size_t len,
size_t *retlen, const u_char *buf)
{
int err = 0, lnum, offs, bytes_left;
- struct gluebi_device *gluebi;
+ struct gluebi_device *gluebi = container_of(mtd, struct gluebi_device,
+ mtd);
+ int not_get = IS_ERR_OR_NULL(gluebi->desc);
+
+ if (not_get) {
+ err = __get_mtd_device(mtd);
+ if (err) {
+ err_msg("cannot get MTD device %d, UBI device %d, volume %d, error %d",
+ mtd->index, gluebi->ubi_num, gluebi->vol_id, err);
+ return err;
+ }
+ }
- gluebi = container_of(mtd, struct gluebi_device, mtd);
lnum = div_u64_rem(to, mtd->erasesize, &offs);
if (len % mtd->writesize || offs % mtd->writesize)
@@ -220,6 +243,9 @@ static int gluebi_write(struct mtd_info *mtd, loff_t
to, size_t len,
}
*retlen = len - bytes_left;
+
+ if (not_get)
+ __put_mtd_device(mtd);
return err;
}
@@ -234,14 +260,24 @@ static int gluebi_write(struct mtd_info *mtd,
loff_t to, size_t len,
static int gluebi_erase(struct mtd_info *mtd, struct erase_info *instr)
{
int err, i, lnum, count;
- struct gluebi_device *gluebi;
+ struct gluebi_device *gluebi = container_of(mtd, struct gluebi_device,
+ mtd);
+ int not_get = IS_ERR_OR_NULL(gluebi->desc);
+
+ if (not_get) {
+ err = __get_mtd_device(mtd);
+ if (err) {
+ err_msg("cannot get MTD device %d, UBI device %d, volume %d, error %d",
+ mtd->index, gluebi->ubi_num, gluebi->vol_id, err);
+ return err;
+ }
+ }
if (mtd_mod_by_ws(instr->addr, mtd) || mtd_mod_by_ws(instr->len, mtd))
return -EINVAL;
lnum = mtd_div_by_eb(instr->addr, mtd);
count = mtd_div_by_eb(instr->len, mtd);
- gluebi = container_of(mtd, struct gluebi_device, mtd);
for (i = 0; i < count - 1; i++) {
err = ubi_leb_unmap(gluebi->desc, lnum + i);
@@ -259,10 +295,14 @@ static int gluebi_erase(struct mtd_info *mtd,
struct erase_info *instr)
if (err)
goto out_err;
+ if (not_get)
+ __put_mtd_device(mtd);
return 0;
out_err:
instr->fail_addr = (long long)lnum * mtd->erasesize;
+ if (not_get)
+ __put_mtd_device(mtd);
return err;
}
--
2.31.1
Powered by blists - more mailing lists