On Thu, Jun 06, 2024 at 06:07:22PM GMT, Avnish Chouhan wrote:
> grub-ofpathname doesn't work with fibre channel because there is no
> function currently implemented for it.
> This patch enables it by prividing a function that looks for the port
> name, building the entire path for OF devices.
>
> Signed-off-by: Diego Domingos <dieg...@br.ibm.com>
> Signed-off-by: Avnish Chouhan <avn...@linux.ibm.com>
> ---
> grub-core/osdep/linux/ofpath.c | 49
> ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 49 insertions(+)
>
> diff --git a/grub-core/osdep/linux/ofpath.c b/grub-core/osdep/linux/ofpath.c
> index a6153d35954..0f5d54e9f2d 100644
> --- a/grub-core/osdep/linux/ofpath.c
> +++ b/grub-core/osdep/linux/ofpath.c
> @@ -350,6 +350,38 @@ of_path_of_ide(const char *sys_devname
> __attribute__((unused)), const char *devi
> return ret;
> }
>
> +
> +static void
> +of_fc_port_name (const char *path, const char *subpath, char *port_name)
> +{
> + char *bname, *basepath, *p;
> + int fd;
> +
> + bname = xmalloc (sizeof (char) * 150);
Why not `char bname[150]` for simplicty ? Is there any reason behind the magic
number
150?
> + basepath = xmalloc (strlen (path));
> +
> + /* Generate the path to get port name information from the drive */
> + strncpy (basepath, path, subpath-path);
> + basepath[subpath-path - 1] = '\0';
I think it can be replaced by `strndup()` for simplicity.
> + p = get_basename (basepath);
> + snprintf (bname, sizeof (char) * 150, "%s/fc_transport/%s/port_name",
> basepath, p);
> +
> + /* Read the information from the port name */
> + fd = open (bname, O_RDONLY);
> + if (fd < 0)
> + grub_util_error (_("cannot open `%s': %s"), bname, strerror (errno));
> +
> + if (read (fd, port_name, sizeof (char) *19) < 0)
> + grub_util_error (_("cannot read `%s': %s"), bname, strerror (errno));
The caller has the port_name's size set to `sizeof (char) * 17`, the
read() may really be overreading, and overflowing port_name.
Moreover the function didn't have a way to check to size of port_name.
> +
> + sscanf (port_name, "0x%s", port_name);
> +
> + close (fd);
> +
> + free (bname);
> + free (basepath);
> +}
> +
> #ifdef __sparc__
> static char *
> of_path_of_nvme(const char *sys_devname __attribute__((unused)),
> @@ -577,6 +609,16 @@ of_path_of_scsi(const char *sys_devname
> __attribute__((unused)), const char *dev
> digit_string = trailing_digits (device);
> if (strncmp (of_path, "/vdevice/", sizeof ("/vdevice/") - 1) == 0)
> {
> + if (strstr (of_path, "vfc-client"))
> + {
> + char * port_name = xmalloc (sizeof (char) * 17);
Again why not `char port_name[17]` ? And lacking of description for
magic number 17.
> + of_fc_port_name (sysfs_path, p, port_name);
Please add an argument, port_name_size, for boundary checking in
of_fc_port_name().
> +
> + snprintf (disk, sizeof (disk), "/%s@%s", disk_name, port_name);
> + free (port_name);
> + }
> + else
> + {
> unsigned long id = 0x8000 | (tgt << 8) | (bus << 5) | lun;
> if (*digit_string == '\0')
> {
> @@ -590,6 +632,13 @@ of_path_of_scsi(const char *sys_devname
> __attribute__((unused)), const char *dev
> snprintf(disk, sizeof (disk),
> "/%s@%04lx000000000000:%c", disk_name, id, 'a' + (part - 1));
> }
> + }
> + } else if (strstr (of_path, "fibre-channel") || (strstr (of_path,
> "vfc-client"))){
> + char * port_name = xmalloc (sizeof (char) * 17);
> + of_fc_port_name (sysfs_path, p, port_name);
> +
> + snprintf (disk, sizeof (disk), "/%s@%s", disk_name, port_name);
> + free (port_name);
Ditto.
Thanks,
Michael
> }
> else
> {
>
> _______________________________________________
> 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
