linux: add in labels for block2mtd

This backports the upstream label feature in block2mtd to the 5.10.x kernel in 
22.03:

https://github.com/torvalds/linux/blob/master/drivers/mtd/devices/block2mtd.c

On Tue, Nov 29, 2022 at 10:23:48AM -0500, Peter Naulls wrote:
> 
> This backports the upstream label feature in block2mtd to the 5.10.x kernel
> in 22.03:
> 
> https://github.com/torvalds/linux/blob/master/drivers/mtd/devices/block2mtd.c

Where are we using block2mtd and why?

I'm not against backporting this, but I don't understand why it is
needed or where we are even using block2mtd.


> --- a/drivers/mtd/devices/block2mtd.c	2022-11-29 07:35:32.382695321 -0500
> +++ b/drivers/mtd/devices/block2mtd.c	2022-11-29 08:04:27.406754981 -0500
> @@ -31,6 +31,9 @@
>  #include <linux/slab.h>
>  #include <linux/major.h>
>  
> +/* Maximum number of comma-separated items in the 'block2mtd=' parameter */
> +#define BLOCK2MTD_PARAM_MAX_COUNT 3
> +
>  /* Info for the block device */
>  struct block2mtd_dev {
>  	struct list_head list;
> @@ -214,7 +217,7 @@
>  
>  
>  static struct block2mtd_dev *add_device(char *devname, int erase_size,
> -		int timeout)
> +		char *label, int timeout)
>  {
>  #ifndef MODULE
>  	int i;
> @@ -278,7 +281,10 @@
>  
>  	/* Setup the MTD structure */
>  	/* make the name contain the block device in */
> -	name = kasprintf(GFP_KERNEL, "block2mtd: %s", devname);
> +	if (!label)
> +		name = kasprintf(GFP_KERNEL, "block2mtd: %s", devname);
> +	else
> +		name = kstrdup(label, GFP_KERNEL);
>  	if (!name)
>  		goto err_destroy_mutex;
>  
> @@ -305,7 +311,7 @@
>  	list_add(&dev->list, &blkmtd_device_list);
>  	pr_info("mtd%d: [%s] erase_size = %dKiB [%d]\n",
>  		dev->mtd.index,
> -		dev->mtd.name + strlen("block2mtd: "),
> +		label ? label : dev->mtd.name + strlen("block2mtd: "),
>  		dev->mtd.erasesize >> 10, dev->mtd.erasesize);
>  	return dev;
>  
> @@ -381,8 +387,9 @@
>  	/* 80 for device, 12 for erase size, 80 for name, 8 for timeout */
>  	char buf[80 + 12 + 80 + 8];
>  	char *str = buf;
> -	char *token[2];
> +	char *token[BLOCK2MTD_PARAM_MAX_COUNT];
>  	char *name;
> +	char *label = NULL;
>  	size_t erase_size = PAGE_SIZE;
>  	unsigned long timeout = MTD_DEFAULT_TIMEOUT;
>  	int i, ret;
> @@ -395,7 +402,7 @@
>  	strcpy(str, val);
>  	kill_final_newline(str);
>  
> -	for (i = 0; i < 2; i++)
> +	for (i = 0; i < BLOCK2MTD_PARAM_MAX_COUNT; i++)
>  		token[i] = strsep(&str, ",");
>  
>  	if (str) {
> @@ -414,7 +421,8 @@
>  		return 0;
>  	}
>  
> -	if (token[1]) {
> +	/* Optional argument when custom label is used */
> +	if (token[1] && strlen(token[1])) {
>  		ret = parse_num(&erase_size, token[1]);
>  		if (ret) {
>  			pr_err("illegal erase size\n");
> @@ -422,7 +430,12 @@
>  		}
>  	}
>  
> -	add_device(name, erase_size, timeout);
> +	if (token[2]) {
> +		label = token[2];
> +		pr_info("Using custom MTD label '%s' for dev %s\n", label, name);
> +	}
> +
> +	add_device(name, erase_size, label, timeout);
>  
>  	return 0;
>  }
> @@ -448,7 +461,7 @@
>  	   the device (even kmalloc() fails). Deter that work to
>  	   block2mtd_setup2(). */
>  
> -	strlcpy(block2mtd_paramline, val, sizeof(block2mtd_paramline));
> +	strscpy(block2mtd_paramline, val, sizeof(block2mtd_paramline));
>  
>  	return 0;
>  #endif
> @@ -456,7 +469,7 @@
>  
>  
>  module_param_call(block2mtd, block2mtd_setup, NULL, NULL, 0200);
> -MODULE_PARM_DESC(block2mtd, "Device to use. \"block2mtd=<dev>[,<erasesize>]\"");
> +MODULE_PARM_DESC(block2mtd, "Device to use. \"block2mtd=<dev>[,[<erasesize>][,<label>]]\"");
>  
>  static int __init block2mtd_init(void)
>  {

> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel

On 11/29/22 10:32, Daniel Golle wrote:
> On Tue, Nov 29, 2022 at 10:23:48AM -0500, Peter Naulls wrote:
>>
>> This backports the upstream label feature in block2mtd to the 5.10.x kernel
>> in 22.03:
>>
>> https://github.com/torvalds/linux/blob/master/drivers/mtd/devices/block2mtd.c
> 
> Where are we using block2mtd and why?
> 

I should have added more context.  I don't think there's really a "we" here,
this is something I needed, and it's more for discussion than anything.  I don't
think it has a general use in OpenWrt at present, and given the release status
of 22.03 you could even argue it shouldn't go in.

My application is for encrypting the rootfs_data partition to meet security
audit requirements (rootfs too, but that's a different step).  I know there
hasn't been much appetite for this in the past, and I'm painfully aware of the
OSS nature here vs encryption, but here we are. This is a requirement for
our product, whether I get pushback or not.

In any case, block2mtd allows me to present devices from cryptsetup to jffs2.
I'm working on some additional patches to make this all work with 'mount_root'
and sysupgrade, so we'll see - it will be experimental in nature for sure, and
may not ultimately be the best way to do things. That's OK.

As for the patch, it'll come to OpenWrt eventually, but my preference is
to stick with some sense of stability with 22.03.

Thanks!

On Tue, Nov 29, 2022 at 11:28:29AM -0500, Peter Naulls wrote:
> On 11/29/22 10:32, Daniel Golle wrote:
> > On Tue, Nov 29, 2022 at 10:23:48AM -0500, Peter Naulls wrote:
> > > 
> > > This backports the upstream label feature in block2mtd to the 5.10.x kernel
> > > in 22.03:
> > > 
> > > https://github.com/torvalds/linux/blob/master/drivers/mtd/devices/block2mtd.c
> > 
> > Where are we using block2mtd and why?
> > 
> 
> I should have added more context.  I don't think there's really a "we" here,
> this is something I needed, and it's more for discussion than anything.  I don't
> think it has a general use in OpenWrt at present, and given the release status
> of 22.03 you could even argue it shouldn't go in.
> 
> My application is for encrypting the rootfs_data partition to meet security
> audit requirements (rootfs too, but that's a different step).  I know there
> hasn't been much appetite for this in the past, and I'm painfully aware of the
> OSS nature here vs encryption, but here we are. This is a requirement for
> our product, whether I get pushback or not.
> 
> In any case, block2mtd allows me to present devices from cryptsetup to jffs2.
> I'm working on some additional patches to make this all work with 'mount_root'
> and sysupgrade, so we'll see - it will be experimental in nature for sure, and
> may not ultimately be the best way to do things. That's OK.

There is nothing wrong with that use-case, and it can even be
interesting for other downstream users. Encrypted rootfs_data is
generally a good idea, especially when rootfs_data is used to store
private key material (think: VPN keys) or other kind of credentials.

I was more wondering why you are using JFFS2 on a block device, instead
of e.g. using F2FS or EXT4 which are intended for block devices.

On 11/29/22 11:50, Daniel Golle wrote:

> 
> There is nothing wrong with that use-case, and it can even be
> interesting for other downstream users. Encrypted rootfs_data is
> generally a good idea, especially when rootfs_data is used to store
> private key material (think: VPN keys) or other kind of credentials.
> 
> I was more wondering why you are using JFFS2 on a block device, instead
> of e.g. using F2FS or EXT4 which are intended for block devices.

Our flash is NOR.  We will probably move to NAND in the next iteration of
hardware, but this is what we have for now.

I'm open to other ways to make it work, but this is the arrangement that
I was able to make work in my research and testing, and that a colleague
used successfully on a non-OpenWrt system.

On Tue, Nov 29, 2022 at 12:05:51PM -0500, Peter Naulls wrote:
> On 11/29/22 11:50, Daniel Golle wrote:
> 
> > 
> > There is nothing wrong with that use-case, and it can even be
> > interesting for other downstream users. Encrypted rootfs_data is
> > generally a good idea, especially when rootfs_data is used to store
> > private key material (think: VPN keys) or other kind of credentials.
> > 
> > I was more wondering why you are using JFFS2 on a block device, instead
> > of e.g. using F2FS or EXT4 which are intended for block devices.
> 
> Our flash is NOR.  We will probably move to NAND in the next iteration of
> hardware, but this is what we have for now.
> 
> I'm open to other ways to make it work, but this is the arrangement that
> I was able to make work in my research and testing, and that a colleague
> used successfully on a non-OpenWrt system.

Ok, that makes sense then. So basically you are basically using
mtd->mtdblock->cryptsetup/luks->block2mtd->jffs2

I thought you are on a device with actual block storage.
For your case I also can't come up with anything better which works
out-of-the-box for NOR flash. Supporting fscrypt in JFFS2 would be more
elegant, but that's a bit more demanding than just using what is
already there and works...

On 11/29/22 12:37, Daniel Golle wrote:

> 
> I thought you are on a device with actual block storage.
> For your case I also can't come up with anything better which works
> out-of-the-box for NOR flash. Supporting fscrypt in JFFS2 would be more
> elegant, but that's a bit more demanding than just using what is
> already there and works...

Well, my big concern here is performance on a small device. I haven't
done much testing yet, but some operations seem quite slow; I won't
really know until I get it all going.  The format of the ~17MB roofs_data
takes 3-4 minutes, but that's a one-time operation after sysupgrade.

In my colleague's case, he used AES and was able to make use of the hardware
acceleration.  I imagine that fscrypt in JFFS2 would avoid a lot of overhead
too.