On Mon, Oct 11, 2021 at 09:30:25AM +0200, Sascha Hauer wrote:
> Until now it has been possible to create overlapping partitions. Go away
> from that and allow to create partitions only in unallocated areas of a
> device. This lowers the risk of having inconsistent partitioning and
> increases the chance that inconsistent partitioning is recognized by
> the user.
>
> We had explicit overlap checking for the environment partition which
> becomes unnecessary with this change and is removed.
>
> Signed-off-by: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
This is now commit 7f9f45b9bfef2ce8706f25c46dcbd109e4181164. Still some
feedback:
> ---
> common/startup.c | 70 +-----------------------------------------------
> fs/devfs-core.c | 34 +++++++++++++++++++++++
> 2 files changed, 35 insertions(+), 69 deletions(-)
>
> diff --git a/common/startup.c b/common/startup.c
> index f72902fc53..f53b73f81a 100644
> --- a/common/startup.c
> +++ b/common/startup.c
> @@ -68,70 +68,6 @@ static int mount_root(void)
> fs_initcall(mount_root);
> #endif
>
> -#ifdef CONFIG_ENV_HANDLING
> -static bool region_overlap(loff_t starta, loff_t lena,
> - loff_t startb, loff_t lenb)
> -{
> - if (starta + lena <= startb)
> - return 0;
> - if (startb + lenb <= starta)
> - return 0;
> - return 1;
> -}
> -
> -static int check_overlap(const char *path)
> -{
> - struct cdev *cenv, *cdisk, *cpart;
> - const char *name;
> -
> - name = devpath_to_name(path);
> -
> - if (name == path)
> - /*
> - * no /dev/ in front, so *path is some file. No need to
> - * check further.
> - */
> - return 0;
> -
> - cenv = cdev_by_name(name);
> - if (!cenv)
> - return -EINVAL;
> -
> - if (cenv->mtd)
> - return 0;
> -
> - cdisk = cenv->master;
> -
> - if (!cdisk)
> - return 0;
> -
> - list_for_each_entry(cpart, &cdisk->partitions, partition_entry) {
> - if (cpart == cenv)
> - continue;
> -
> - if (region_overlap(cpart->offset, cpart->size,
> - cenv->offset, cenv->size))
> - goto conflict;
> - }
> -
> - return 0;
> -
> -conflict:
> - pr_err("Environment partition (0x%08llx-0x%08llx) "
> - "overlaps with partition %s (0x%08llx-0x%08llx), not using it\n",
> - cenv->offset, cenv->offset + cenv->size - 1,
> - cpart->name,
> - cpart->offset, cpart->offset + cpart->size - 1);
> -
> - return -EINVAL;
> -}
> -#else
> -static int check_overlap(const char *path)
> -{
> - return 0;
> -}
> -#endif
> -
> static int load_environment(void)
> {
> const char *default_environment_path;
> @@ -143,11 +79,7 @@ static int load_environment(void)
> defaultenv_load("/env", 0);
>
> if (IS_ENABLED(CONFIG_ENV_HANDLING)) {
> - ret = check_overlap(default_environment_path);
> - if (ret)
> - default_environment_path_set(NULL);
> - else
> - envfs_load(default_environment_path, "/env", 0);
> + envfs_load(default_environment_path, "/env", 0);
> } else {
> if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT))
> pr_notice("No support for persistent environment. Using default environment\n");
> diff --git a/fs/devfs-core.c b/fs/devfs-core.c
> index 30ad0e0508..3715e543e6 100644
> --- a/fs/devfs-core.c
> +++ b/fs/devfs-core.c
> @@ -301,6 +301,37 @@ int devfs_remove(struct cdev *cdev)
> return 0;
> }
>
> +static bool region_overlap(loff_t starta, loff_t lena,
> + loff_t startb, loff_t lenb)
> +{
> + if (starta + lena <= startb)
> + return 0;
> + if (startb + lenb <= starta)
> + return 0;
> + return 1;
> +}
> +
> +static int check_overlap(struct cdev *cdev, const char *name, loff_t offset, loff_t size)
> +{
> + struct cdev *cpart;
> +
> + list_for_each_entry(cpart, &cdev->partitions, partition_entry) {
> + if (region_overlap(cpart->offset, cpart->size,
> + offset, size))
> + goto conflict;
> + }
> +
> + return 0;
> +
> +conflict:
> + pr_err("New partition %s (0x%08llx-0x%08llx) on %s "
> + "overlaps with partition %s (0x%08llx-0x%08llx), not creating it\n",
> + name, offset, offset + size - 1, cpart->name,
> + cpart->name, cpart->offset, cpart->offset + cpart->size - 1);
The first cpart->name should be cdev->name.
This warning wrongly triggers for a machine after updating 2021.07.0 ->
2022.05.0 here:
bootloader: echo $nand0.partitions
1M(barebox),1M(environment),80M@4480k(root),44672k(var)
bootloader: addpart /dev/nand0 0x260000@0x00200000(history)
nand0: __devfs_add_partition:422: offset=200000, size=260000, name=nand0.history
New partition nand0.history (0x00200000-0x0045ffff) on nand0 overlaps with partition nand0.root (0x00000000-0x04ffffff), not creating it
cannot create nand0.history: Invalid argument
addpart: Invalid argument
The problem seems to be that for mtd partitions(?) the offset is 0 while
in reality the offset of the "root" partition is 4480*1024 = 0x460000
(relative to the master mtd device at least).
I didn't test, but I guess
- if (cenv->mtd)
- return 0;
in the old check_overlap function made this problem not appear before
7f9f45b9bfef2ce8706f25c46dcbd109e4181164.
Do you have a suggestion how to best fix this?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
