[PATCH] dm: Allow the use of escaped characters in str_field_delimit()

[PATCH] dm: Allow the use of escaped characters in str_field_delimit()

[Mohammed Anees]

Escape characters were not handled before, which could lead to
unwanted issues. Some device-mapper names may contain backslashes (`\`)
as valid characters and should not be treated as escape characters. Only
escape characters followed directly by the separator are considered
valid and need to be processed. After handling, the escape characters
are removed to ensure the final string is correctly parsed without
unwanted escape sequences which were used only for escaping.

Signed-off-by: Mohammed Anees <pvmohammedanees2003@gmail.com>
---
 drivers/md/dm-init.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/md/dm-init.c b/drivers/md/dm-init.c
index b37bbe762500..dad9d523f7fb 100644
--- a/drivers/md/dm-init.c
+++ b/drivers/md/dm-init.c
@@ -88,13 +88,33 @@ static void __init dm_setup_cleanup(struct list_head *devices)
 static char __init *str_field_delimit(char **str, char separator)
 {
 	char *s;
+	/* This variable handles removing escape characters, which are
+	 * only used to avoid the separator and aren't needed in the
+	 * final string.
+	 */
+	char *write;
 
-	/* TODO: add support for escaped characters */
 	*str = skip_spaces(*str);
-	s = strchr(*str, separator);
+	s = *str;
+	write = *str;
+
+	/* Find the separator and handle escape character */
+	while (*s) {
+		/* If '\' is followed by the separator, skip '\' by
+		 * incrementing s, write will then overwrite the
+		 * escape character with the separator.
+		 */
+		if (*s == '\\' && *(s + 1) != '\0' && *(s + 1) == separator)
+			s++;
+		else if (*s == separator)
+			break;
+
+		*write++ = *s++;
+	}
+
 	/* Delimit the field and remove trailing spaces */
-	if (s)
-		*s = '\0';
+	if (write)
+		*write = '\0';
 	*str = strim(*str);
 	return s ? ++s : NULL;
 }
-- 
2.47.0

Re: [PATCH] dm: Allow the use of escaped characters in str_field_delimit()

[Zdenek Kabelac]

Dne 12. 11. 24 v 18:57 Mohammed Anees napsal(a):
> Escape characters were not handled before, which could lead to
> unwanted issues. Some device-mapper names may contain backslashes (`\`)
> as valid characters and should not be treated as escape characters. Only
> escape characters followed directly by the separator are considered
> valid and need to be processed. After handling, the escape characters
> are removed to ensure the final string is correctly parsed without
> unwanted escape sequences which were used only for escaping.
>
> Signed-off-by: Mohammed Anees <pvmohammedanees2003@gmail.com>
> ---
>   drivers/md/dm-init.c | 28 ++++++++++++++++++++++++----
>   1 file changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/md/dm-init.c b/drivers/md/dm-init.c
> index b37bbe762500..dad9d523f7fb 100644
> --- a/drivers/md/dm-init.c
> +++ b/drivers/md/dm-init.c
> @@ -88,13 +88,33 @@ static void __init dm_setup_cleanup(struct list_head *devices)
>   static char __init *str_field_delimit(char **str, char separator)
>   {
>

There is libdevmapper  project  (ATM distirbuted through lvm2 project) which 
is handling the management of name & uuid with mangling - so they are properly 
visible on systems with udev.

IMHO this escape handling does not belong to kernel and is rather related to 
the actual user space running on top.


Regards


Zdenek

Re: [PATCH] dm: Allow the use of escaped characters in str_field_delimit()

[Mikulas Patocka]

On Tue, 12 Nov 2024, Mohammed Anees wrote:

> Escape characters were not handled before, which could lead to
> unwanted issues. Some device-mapper names may contain backslashes (`\`)
> as valid characters and should not be treated as escape characters. Only
> escape characters followed directly by the separator are considered
> valid and need to be processed. After handling, the escape characters
> are removed to ensure the final string is correctly parsed without
> unwanted escape sequences which were used only for escaping.
> 
> Signed-off-by: Mohammed Anees <pvmohammedanees2003@gmail.com>

Hi

Does anyone really need this? Is there some use case for using escape 
characters in device mapper names?

Mikulas

> ---
>  drivers/md/dm-init.c | 28 ++++++++++++++++++++++++----
>  1 file changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/md/dm-init.c b/drivers/md/dm-init.c
> index b37bbe762500..dad9d523f7fb 100644
> --- a/drivers/md/dm-init.c
> +++ b/drivers/md/dm-init.c
> @@ -88,13 +88,33 @@ static void __init dm_setup_cleanup(struct list_head *devices)
>  static char __init *str_field_delimit(char **str, char separator)
>  {
>  	char *s;
> +	/* This variable handles removing escape characters, which are
> +	 * only used to avoid the separator and aren't needed in the
> +	 * final string.
> +	 */
> +	char *write;
>  
> -	/* TODO: add support for escaped characters */
>  	*str = skip_spaces(*str);
> -	s = strchr(*str, separator);
> +	s = *str;
> +	write = *str;
> +
> +	/* Find the separator and handle escape character */
> +	while (*s) {
> +		/* If '\' is followed by the separator, skip '\' by
> +		 * incrementing s, write will then overwrite the
> +		 * escape character with the separator.
> +		 */
> +		if (*s == '\\' && *(s + 1) != '\0' && *(s + 1) == separator)
> +			s++;
> +		else if (*s == separator)
> +			break;
> +
> +		*write++ = *s++;
> +	}
> +
>  	/* Delimit the field and remove trailing spaces */
> -	if (s)
> -		*s = '\0';
> +	if (write)
> +		*write = '\0';
>  	*str = strim(*str);
>  	return s ? ++s : NULL;
>  }
> -- 
> 2.47.0
> 

Re: dm: Allow the use of escaped characters in str_field_delimit()

[Mike Snitzer]

On Mon, Nov 18, 2024 at 11:38:05AM +0100, Mikulas Patocka wrote:
> 
> 
> On Tue, 12 Nov 2024, Mohammed Anees wrote:
> 
> > Escape characters were not handled before, which could lead to
> > unwanted issues. Some device-mapper names may contain backslashes (`\`)
> > as valid characters and should not be treated as escape characters. Only
> > escape characters followed directly by the separator are considered
> > valid and need to be processed. After handling, the escape characters
> > are removed to ensure the final string is correctly parsed without
> > unwanted escape sequences which were used only for escaping.
> > 
> > Signed-off-by: Mohammed Anees <pvmohammedanees2003@gmail.com>
> 
> Hi
> 
> Does anyone really need this? Is there some use case for using escape 
> characters in device mapper names?

It would seem Mohammed cared enough to write the patch, but not reply
to you with further clarification on why it needed...

BUT, in this instance it follows that: if lvm2 is allowing weird names
which require escacped characters _and_ dm-init is used then dm-init
needs to support handling them (dm-init is all about _not_ using
normal initramfs with lvm2 in all its glory).

Mike

Re: dm: Allow the use of escaped characters in str_field_delimit()

[Mohammed Anees]

> It would seem Mohammed cared enough to write the patch, but not reply
> to you with further clarification on why it needed...

I thought I had already replied to the concern, but it seems 
the message didn’t get sent, apologies for that oversight!
College academics kept me tied up, and I lost track of this,
apologies once again.

> BUT, in this instance it follows that: if lvm2 is allowing weird names
> which require escacped characters _and_ dm-init is used then dm-init
> needs to support handling them (dm-init is all about _not_ using
> normal initramfs with lvm2 in all its glory).

I completely agree with your point and am more than happy to 
provide further details or make any additional updates to 
the patch if needed, please let me know if anything else if needed. 
Thank you for your time.

Regards
Mohammed Anees

Re: dm: Allow the use of escaped characters in str_field_delimit()

[Mikulas Patocka]

[-- Attachment #1: Type: text/plain, Size: 1141 bytes --]



On Sun, 15 Dec 2024, Mohammed Anees wrote:

> > It would seem Mohammed cared enough to write the patch, but not reply
> > to you with further clarification on why it needed...
> 
> I thought I had already replied to the concern, but it seems 
> the message didn’t get sent, apologies for that oversight!
> College academics kept me tied up, and I lost track of this,
> apologies once again.
> 
> > BUT, in this instance it follows that: if lvm2 is allowing weird names
> > which require escacped characters _and_ dm-init is used then dm-init
> > needs to support handling them (dm-init is all about _not_ using
> > normal initramfs with lvm2 in all its glory).
> 
> I completely agree with your point and am more than happy to 
> provide further details or make any additional updates to 
> the patch if needed, please let me know if anything else if needed. 
> Thank you for your time.
> 
> Regards
> Mohammed Anees

Hi

I don't want to bloat the code with the logic that no one uses. I would 
only accept the patch if there were some real scenario where you need to 
use escaped characters in device names or table parameters.

Mikulas

Re: dm: Allow the use of escaped characters in str_field_delimit()

[Mike Snitzer]

On Fri, Jan 03, 2025 at 05:21:13PM +0100, Mikulas Patocka wrote:
> 
> 
> On Sun, 15 Dec 2024, Mohammed Anees wrote:
> 
> > > It would seem Mohammed cared enough to write the patch, but not reply
> > > to you with further clarification on why it needed...
> > 
> > I thought I had already replied to the concern, but it seems 
> > the message didn´t get sent, apologies for that oversight!
> > College academics kept me tied up, and I lost track of this,
> > apologies once again.
> > 
> > > BUT, in this instance it follows that: if lvm2 is allowing weird names
> > > which require escacped characters _and_ dm-init is used then dm-init
> > > needs to support handling them (dm-init is all about _not_ using
> > > normal initramfs with lvm2 in all its glory).
> > 
> > I completely agree with your point and am more than happy to 
> > provide further details or make any additional updates to 
> > the patch if needed, please let me know if anything else if needed. 
> > Thank you for your time.
> > 
> > Regards
> > Mohammed Anees
> 
> Hi
> 
> I don't want to bloat the code with the logic that no one uses. I would 
> only accept the patch if there were some real scenario where you need to 
> use escaped characters in device names or table parameters.

Please read my reply in this thread (which you quoted above).  The
real scenario was already answered.

Either:

1) remove dm-init entirely

or

2) fix it so that it works with naming that requires escaped
   characters 
or

3) say: "sorry, dm-init doesn't support escaped characters."