[PATCH] * grub-core/fs/udf.c: Fix reading label, lvd.ident is dstring

[PATCH] * grub-core/fs/udf.c: Fix reading label, lvd.ident is dstring

[Pali Rohár]

UDF dstring has stored length in the last byte of buffer. Therefore last
byte is not part of recorded characters. And empty string in dstring is
encoded as empty buffer, including first byte (compression id).
---
I'm not sure how Grub2 should handle empty label. Current patch set
empty buffer for empty label. But it is possible to ignore and do not
set label in this case at all.
---
 grub-core/fs/udf.c |   21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/grub-core/fs/udf.c b/grub-core/fs/udf.c
index 839bff8..e7a4d4e 100644
--- a/grub-core/fs/udf.c
+++ b/grub-core/fs/udf.c
@@ -860,6 +860,25 @@ read_string (const grub_uint8_t *raw, grub_size_t sz, char *outbuf)
   return outbuf;
 }
 
+static char *
+read_dstring (const grub_uint8_t *raw, grub_size_t sz, char *outbuf)
+{
+  grub_size_t len;
+
+  if (raw[0] == 0)
+    {
+      if (!outbuf)
+        outbuf = grub_malloc (1);
+      outbuf[0] = 0;
+      return outbuf;
+    }
+
+  len = raw[sz - 1];
+  if (len > sz - 1)
+    len = sz - 1;
+  return read_string (raw, len, outbuf);
+}
+
 static int
 grub_udf_iterate_dir (grub_fshelp_node_t dir,
 		      grub_fshelp_iterate_dir_hook_t hook, void *hook_data)
@@ -1197,7 +1216,7 @@ grub_udf_label (grub_device_t device, char **label)
 
   if (data)
     {
-      *label = read_string (data->lvd.ident, sizeof (data->lvd.ident), 0);
+      *label = read_dstring (data->lvd.ident, sizeof (data->lvd.ident), 0);
       grub_free (data);
     }
   else
-- 
1.7.9.5

Re: [PATCH] * grub-core/fs/udf.c: Fix reading label, lvd.ident is dstring

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 1813 bytes --]

On Thursday 22 June 2017 14:33:17 Pali Rohár wrote:
> UDF dstring has stored length in the last byte of buffer. Therefore
> last byte is not part of recorded characters. And empty string in
> dstring is encoded as empty buffer, including first byte
> (compression id).
> ---
> I'm not sure how Grub2 should handle empty label. Current patch set
> empty buffer for empty label. But it is possible to ignore and do not
> set label in this case at all.
> ---

Hi! Any comments on this patch? Specially how to deal with empty labels?

>  grub-core/fs/udf.c |   21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/grub-core/fs/udf.c b/grub-core/fs/udf.c
> index 839bff8..e7a4d4e 100644
> --- a/grub-core/fs/udf.c
> +++ b/grub-core/fs/udf.c
> @@ -860,6 +860,25 @@ read_string (const grub_uint8_t *raw,
> grub_size_t sz, char *outbuf) return outbuf;
>  }
> 
> +static char *
> +read_dstring (const grub_uint8_t *raw, grub_size_t sz, char *outbuf)
> +{
> +  grub_size_t len;
> +
> +  if (raw[0] == 0)
> +    {
> +      if (!outbuf)
> +        outbuf = grub_malloc (1);
> +      outbuf[0] = 0;
> +      return outbuf;
> +    }
> +
> +  len = raw[sz - 1];
> +  if (len > sz - 1)
> +    len = sz - 1;
> +  return read_string (raw, len, outbuf);
> +}
> +
>  static int
>  grub_udf_iterate_dir (grub_fshelp_node_t dir,
>  		      grub_fshelp_iterate_dir_hook_t hook, void *hook_data)
> @@ -1197,7 +1216,7 @@ grub_udf_label (grub_device_t device, char
> **label)
> 
>    if (data)
>      {
> -      *label = read_string (data->lvd.ident, sizeof
> (data->lvd.ident), 0); +      *label = read_dstring
> (data->lvd.ident, sizeof (data->lvd.ident), 0); grub_free (data);
>      }
>    else

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] * grub-core/fs/udf.c: Fix reading label, lvd.ident is dstring

[Vladimir 'phcoder' Serbinenko]

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

Committed with fixes and simplifications. The biggest problem was lack of
check whether grub_malloc succeeded

Le Thu, Jun 22, 2017 à 2:34 PM, Pali Rohár <pali.rohar@gmail.com> a écrit :

> UDF dstring has stored length in the last byte of buffer. Therefore last
> byte is not part of recorded characters. And empty string in dstring is
> encoded as empty buffer, including first byte (compression id).
> ---
> I'm not sure how Grub2 should handle empty label. Current patch set
> empty buffer for empty label. But it is possible to ignore and do not
> set label in this case at all.
> ---
>  grub-core/fs/udf.c |   21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/grub-core/fs/udf.c b/grub-core/fs/udf.c
> index 839bff8..e7a4d4e 100644
> --- a/grub-core/fs/udf.c
> +++ b/grub-core/fs/udf.c
> @@ -860,6 +860,25 @@ read_string (const grub_uint8_t *raw, grub_size_t sz,
> char *outbuf)
>    return outbuf;
>  }
>
> +static char *
> +read_dstring (const grub_uint8_t *raw, grub_size_t sz, char *outbuf)
> +{
> +  grub_size_t len;
> +
> +  if (raw[0] == 0)
> +    {
> +      if (!outbuf)
> +        outbuf = grub_malloc (1);
> +      outbuf[0] = 0;
> +      return outbuf;
> +    }
> +
> +  len = raw[sz - 1];
> +  if (len > sz - 1)
> +    len = sz - 1;
> +  return read_string (raw, len, outbuf);
> +}
> +
>  static int
>  grub_udf_iterate_dir (grub_fshelp_node_t dir,
>                       grub_fshelp_iterate_dir_hook_t hook, void *hook_data)
> @@ -1197,7 +1216,7 @@ grub_udf_label (grub_device_t device, char **label)
>
>    if (data)
>      {
> -      *label = read_string (data->lvd.ident, sizeof (data->lvd.ident), 0);
> +      *label = read_dstring (data->lvd.ident, sizeof (data->lvd.ident),
> 0);
>        grub_free (data);
>      }
>    else
> --
> 1.7.9.5
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

[-- Attachment #2: Type: text/html, Size: 2626 bytes --]

Re: [PATCH] * grub-core/fs/udf.c: Fix reading label, lvd.ident is dstring

[Pali Rohár]

Thanks a lot!

On Monday 07 August 2017 15:40:15 Vladimir 'phcoder' Serbinenko wrote:
> Committed with fixes and simplifications. The biggest problem was lack of
> check whether grub_malloc succeeded
> 
> Le Thu, Jun 22, 2017 à 2:34 PM, Pali Rohár <pali.rohar@gmail.com> a écrit :
> 
> > UDF dstring has stored length in the last byte of buffer. Therefore last
> > byte is not part of recorded characters. And empty string in dstring is
> > encoded as empty buffer, including first byte (compression id).
> > ---
> > I'm not sure how Grub2 should handle empty label. Current patch set
> > empty buffer for empty label. But it is possible to ignore and do not
> > set label in this case at all.
> > ---
> >  grub-core/fs/udf.c |   21 ++++++++++++++++++++-
> >  1 file changed, 20 insertions(+), 1 deletion(-)
> >
> > diff --git a/grub-core/fs/udf.c b/grub-core/fs/udf.c
> > index 839bff8..e7a4d4e 100644
> > --- a/grub-core/fs/udf.c
> > +++ b/grub-core/fs/udf.c
> > @@ -860,6 +860,25 @@ read_string (const grub_uint8_t *raw, grub_size_t sz,
> > char *outbuf)
> >    return outbuf;
> >  }
> >
> > +static char *
> > +read_dstring (const grub_uint8_t *raw, grub_size_t sz, char *outbuf)
> > +{
> > +  grub_size_t len;
> > +
> > +  if (raw[0] == 0)
> > +    {
> > +      if (!outbuf)
> > +        outbuf = grub_malloc (1);
> > +      outbuf[0] = 0;
> > +      return outbuf;
> > +    }
> > +
> > +  len = raw[sz - 1];
> > +  if (len > sz - 1)
> > +    len = sz - 1;
> > +  return read_string (raw, len, outbuf);
> > +}
> > +
> >  static int
> >  grub_udf_iterate_dir (grub_fshelp_node_t dir,
> >                       grub_fshelp_iterate_dir_hook_t hook, void *hook_data)
> > @@ -1197,7 +1216,7 @@ grub_udf_label (grub_device_t device, char **label)
> >
> >    if (data)
> >      {
> > -      *label = read_string (data->lvd.ident, sizeof (data->lvd.ident), 0);
> > +      *label = read_dstring (data->lvd.ident, sizeof (data->lvd.ident),
> > 0);
> >        grub_free (data);
> >      }
> >    else
> > --
> > 1.7.9.5
> >
> >
> > _______________________________________________
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel
> >

-- 
Pali Rohár
pali.rohar@gmail.com