On Mon, Sep 30, 2024 at 11:21:53AM +0200, Rasmus Villemoes wrote: > Christian Marangi <ansuelsmth@xxxxxxxxx> writes: > > > diff --git a/block/partitions/of.c b/block/partitions/of.c > > new file mode 100644 > > index 000000000000..bc6200eb86b3 > > --- /dev/null > > +++ b/block/partitions/of.c > > @@ -0,0 +1,151 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > + > > +#include <linux/blkdev.h> > > +#include <linux/major.h> > > +#include <linux/of.h> > > +#include "check.h" > > + > > +#define BOOT0_STR "boot0" > > +#define BOOT1_STR "boot1" > > + > > +static struct device_node *get_partitions_node(struct device_node *disk_np, > > + struct gendisk *disk) > > +{ > > + const char *node_name = "partitions"; > > + > > + /* > > + * JEDEC specification 4.4 for eMMC introduced 3 additional partition > > + * present on every eMMC. These additional partition are always hardcoded > > + * from the eMMC driver as boot0, boot1 and rpmb. While rpmb is used to > > + * store keys and exposed as a char device, the other 2 are exposed as > > + * real separate disk with the boot0/1 appended to the disk name. > > + * > > + * Here we parse the disk_name in search for such suffix and select > > + * the correct partition node. > > + */ > > + if (disk->major == MMC_BLOCK_MAJOR) { > > + const char *disk_name = disk->disk_name; > > + > > + if (!memcmp(disk_name + strlen(disk_name) - strlen(BOOT0_STR), > > + BOOT0_STR, sizeof(BOOT0_STR))) > > + node_name = "partitions-boot0"; > > If strlen(disk_name) is less than 5 (and I don't know if that's actually > possible), this well end up doing out-of-bounds access. > > We have a strstarts() helper, could you also add a strends() helper that > handles this correctly? Something like > > /** > * strends - does @str end with @suffix? > * @str: string to examine > * @suffix: suffix to look for. > */ > static inline bool strends(const char *str, const char *suffix) > { > size_t n = strlen(str); > size_t m = strlen(suffix); > return n >= m && !memcmp(str + n - m, suffix, m); > } > > [or name it str_has_suffix() or str_ends_with(), "strends" is not > particularly readable, it's unfortunate that the existing strstarts is > spelled like that]. > Nice idea and thanks for checking the problem with the out-of-bounds read. Out of consistency with the unreadable strstarts I'm tempted to use strends. Since checking suffix of a string can't be something that unreal I searched for the 3 function name and to my surprise all 3 suggested name have a variant of the function statically defined hahaha. To not pollute this series I will just introduce the helper but I will add on my TODO list to convert the other function to make use of this helper instead. -- Ansuel