Re: [PATCH 2/6] ieee1275/powerpc: enables device mapper discovery

[Stefan Berger]

On 3/15/24 00:06, Michael Chang via Grub-devel wrote:
> On Mon, May 08, 2023 at 01:58:36PM +0000, Avnish Chouhan wrote:
> > From: Diego Domingos <diegodo@br.ibm.com>
> >
> > This patch enables the device mapper discovery on ofpath.c. Currently,
> > when we are dealing with a device like /dev/dm-* the ofpath returns null
> > since there is no function implemented to handle this case.
> >
> > This patch implements a function that will look into /sys/block/dm-*
> > devices and search recursively inside slaves directory to find the root
> > disk.
> >
> > Signed-off-by: Diego Domingos <diegodo@br.ibm.com>
> > ---
> > grub-core/osdep/linux/ofpath.c | 64 +++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 63 insertions(+), 1 deletion(-)
> >
> > diff --git a/grub-core/osdep/linux/ofpath.c b/grub-core/osdep/linux/ofpath.c
> > index 0f5d54e9f2d..cc849d9c94c 100644
> > --- a/grub-core/osdep/linux/ofpath.c
> > +++ b/grub-core/osdep/linux/ofpath.c
> > @@ -37,6 +37,7 @@
> > #include <fcntl.h>
> > #include <errno.h>
> > #include <ctype.h>
> > +#include <dirent.h>
> >
> > #ifdef __sparc__
> > typedef enum
> > @@ -755,13 +756,74 @@ strip_trailing_digits (const char *p)
> >    return new;
> > }
> >
> > +static char *
> > +get_slave_from_dm(const char * device){
> > +  char *curr_device, *tmp;
> > +  char *directory;
> > +  char *ret = NULL;
> > +
> > +  directory = grub_strdup (device);

Why do you duplicate the string?

> > +  tmp = get_basename(directory);
> > +  curr_device = grub_strdup (tmp);
> > +  *tmp = '\0';

Is this necessary?

> > +
> > +  /* Recursively check for slaves devices so we can find the root device */
> > +  while ((curr_device[0] == 'd') && (curr_device[1] == 'm') && (curr_device[2] 
> > == '-')){

grub_strncmp(curr_device, "dm-", 3) == 0  ?

> > +    DIR *dp;
> > +    struct dirent *ep;
> > +    char* device_path;
> > +
> > +    device_path = grub_xasprintf ("/sys/block/%s/slaves", curr_device);
> > +    dp = opendir(device_path);
> > +    free(device_path);
> > +
> > +    if (dp != NULL)
> > +      {
> > +        ep = readdir (dp);
> > +        while (ep != NULL)
> > +          {
> > +            /* avoid some system directories */
> > +            if (!strcmp(ep->d_name,"."))
> > +              goto next_dir;
> > +            if (!strcmp(ep->d_name,".."))
> > +              goto next_dir;

if (!strcmp(ep->d_name, ".") || !(strcmp(ep->d_name, ".."))
  {
    ep = readdir(dp);
    continue;
  }

> > +
> > +            free (curr_device);
> > +	     free (ret); >> +	     curr_device = grub_strdup (ep->d_name);>> +	     ret = 
grub_xasprintf ("%s%s", directory, curr_device);
you will still have to test for NULL pointers after grub_strdup() and 
grub_xasprintf() calls.

> > +            break;
> > +
> > +            next_dir:
> > +            ep = readdir (dp);
> > +            continue;
> > +          }
> > +        closedir (dp);
> > +      }
> > +    else
> > +      grub_util_warn (_("cannot open directory `%s'"), device_path);
>
> This will lead to UAF (Use After Free) for the device_path pointer.
>
> Thanks,
> Micahel
>
> > +  }
> > +
> > +  free (directory);
> > +  free (curr_device);
> > +
> > +  return ret;
> > +}
> > +
> > char *
> > grub_util_devname_to_ofpath (const char *sys_devname)
> > {
> > -  char *name_buf, *device, *devnode, *devicenode, *ofpath;
> > +  char *name_buf, *device, *devnode, *devicenode, *ofpath, *realname;
> >
> >    name_buf = xrealpath (sys_devname);
> >
> > +  realname = get_slave_from_dm (name_buf);
> > +  if (realname)
> > +    {
> > +      free (name_buf);
> > +      name_buf = realname;
> > +    }
> > +
> >    device = get_basename (name_buf);
> >    devnode = strip_trailing_digits (name_buf);
> >    devicenode = strip_trailing_digits (device);
> >
> > _______________________________________________
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel