[PATCH 1/3] mtd: initialize partitions_entry

[PATCH 1/3] mtd: initialize partitions_entry

[Sascha Hauer]

&mtd->partitions_entry is added to the parents partition list only when
DEVFS_PARTITION_FIXED is not set, but later this check is not done when
removing it from the list again. This results in NULL pointer derefs
when a mtd partition is added with DEVFS_PARTITION_FIXED set and removed
later.

Do a INIT_LIST_HEAD() on &mtd->partitions_entry so we can safely call
list_del() on it later without additional checks. This means we can
remove the existing check as well.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mtd/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/core.c b/drivers/mtd/core.c
index ae6b0f9cd4..97a7996cf6 100644
--- a/drivers/mtd/core.c
+++ b/drivers/mtd/core.c
@@ -688,6 +688,7 @@ int add_mtd_device(struct mtd_info *mtd, const char *devname, int device_id)
 					     mtd->dev.id);
 
 	INIT_LIST_HEAD(&mtd->partitions);
+	INIT_LIST_HEAD(&mtd->partitions_entry);
 
 	mtd->cdev.priv = mtd;
 	mtd->cdev.dev = &mtd->dev;
@@ -763,8 +764,7 @@ int del_mtd_device(struct mtd_info *mtd)
 	unregister_device(&mtd->dev);
 	free(mtd->param_size.value);
 	free(mtd->cdev.name);
-	if (mtd->parent)
-		list_del(&mtd->partitions_entry);
+	list_del(&mtd->partitions_entry);
 
 	return 0;
 }
-- 
2.39.2

[PATCH 2/3] bootstrap: Fix partition creation

[Sascha Hauer]

bootstrap_read_devfs() registers a partition with DEVFS_PARTITION_FIXED.
The purpose of this flag is that the partition can't be removed later.
Removing the partition is exactly what bootstrap_read_devfs() does when
finished, so remove the DEVFS_PARTITION_FIXED flag which doesn't make
sense here.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 lib/bootstrap/devfs.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/bootstrap/devfs.c b/lib/bootstrap/devfs.c
index 603e6744f2..3d5b1278fb 100644
--- a/lib/bootstrap/devfs.c
+++ b/lib/bootstrap/devfs.c
@@ -88,8 +88,7 @@ void* bootstrap_read_devfs(char *devname, bool use_bb, int offset,
 	struct cdev *cdev, *partition;
 	char *partname = "x";
 
-	partition = devfs_add_partition(devname, offset, max_size,
-					DEVFS_PARTITION_FIXED, partname);
+	partition = devfs_add_partition(devname, offset, max_size, partname);
 	if (IS_ERR(partition)) {
 		bootstrap_err("%s: failed to add partition (%ld)\n",
 			      devname, PTR_ERR(partition));
-- 
2.39.2

[PATCH 3/3] devfs: check for valid flags before removing a partition

[Sascha Hauer]

When a partition is a mtd device then it is removed before checking if
the cdev actually a partition. move the call to mtd_del_partition()
further down to a point where we know the operation is valid.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 fs/devfs-core.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/devfs-core.c b/fs/devfs-core.c
index c9f7fcfb07..4e16d55e36 100644
--- a/fs/devfs-core.c
+++ b/fs/devfs-core.c
@@ -582,16 +582,16 @@ int devfs_del_partition(const char *name)
 	if (!cdev)
 		return -ENOENT;
 
-	if (IS_ENABLED(CONFIG_MTD) && cdev->mtd) {
-		ret = mtd_del_partition(cdev->mtd);
-		return ret;
-	}
-
 	if (!cdev_is_partition(cdev))
 		return -EINVAL;
 	if (cdev->flags & DEVFS_PARTITION_FIXED)
 		return -EPERM;
 
+	if (IS_ENABLED(CONFIG_MTD) && cdev->mtd) {
+		ret = mtd_del_partition(cdev->mtd);
+		return ret;
+	}
+
 	ret = devfs_remove(cdev);
 	if (ret)
 		return ret;
-- 
2.39.2

Re: [PATCH 2/3] bootstrap: Fix partition creation

[Steffen Trumtrar]

On 2023-10-12 at 13:10 +02, Sascha Hauer <s.hauer@pengutronix.de> wrote:

> bootstrap_read_devfs() registers a partition with DEVFS_PARTITION_FIXED.
> The purpose of this flag is that the partition can't be removed later.
> Removing the partition is exactly what bootstrap_read_devfs() does when
> finished, so remove the DEVFS_PARTITION_FIXED flag which doesn't make
> sense here.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  lib/bootstrap/devfs.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/lib/bootstrap/devfs.c b/lib/bootstrap/devfs.c
> index 603e6744f2..3d5b1278fb 100644
> --- a/lib/bootstrap/devfs.c
> +++ b/lib/bootstrap/devfs.c
> @@ -88,8 +88,7 @@ void* bootstrap_read_devfs(char *devname, bool use_bb, int offset,
>  	struct cdev *cdev, *partition;
>  	char *partname = "x";
>
> -	partition = devfs_add_partition(devname, offset, max_size,
> -					DEVFS_PARTITION_FIXED, partname);
> +	partition = devfs_add_partition(devname, offset, max_size, partname);

This accidentally dropped the flags parameter alltogether, which is still needed by the devfs_add_partition declaration, isn't it?


Best regards,
Steffen

--
Pengutronix e.K.                | Dipl.-Inform. Steffen Trumtrar |
Steuerwalder Str. 21            | https://www.pengutronix.de/    |
31137 Hildesheim, Germany       | Phone: +49-5121-206917-0       |
Amtsgericht Hildesheim, HRA 2686| Fax:   +49-5121-206917-5555    |

Re: [PATCH 2/3] bootstrap: Fix partition creation

[Sascha Hauer]

On Thu, Oct 12, 2023 at 01:47:40PM +0200, Steffen Trumtrar wrote:
> 
> On 2023-10-12 at 13:10 +02, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 
> > bootstrap_read_devfs() registers a partition with DEVFS_PARTITION_FIXED.
> > The purpose of this flag is that the partition can't be removed later.
> > Removing the partition is exactly what bootstrap_read_devfs() does when
> > finished, so remove the DEVFS_PARTITION_FIXED flag which doesn't make
> > sense here.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  lib/bootstrap/devfs.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/lib/bootstrap/devfs.c b/lib/bootstrap/devfs.c
> > index 603e6744f2..3d5b1278fb 100644
> > --- a/lib/bootstrap/devfs.c
> > +++ b/lib/bootstrap/devfs.c
> > @@ -88,8 +88,7 @@ void* bootstrap_read_devfs(char *devname, bool use_bb, int offset,
> >  	struct cdev *cdev, *partition;
> >  	char *partname = "x";
> > 
> > -	partition = devfs_add_partition(devname, offset, max_size,
> > -					DEVFS_PARTITION_FIXED, partname);
> > +	partition = devfs_add_partition(devname, offset, max_size, partname);
> 
> This accidentally dropped the flags parameter alltogether, which is still needed by the devfs_add_partition declaration, isn't it?

Yes, I meant to replace DEVFS_PARTITION_FIXED with 0 of course.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |