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 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/ |