[PATCH] discover/udev.c: Added warning in system status log

Jeremy Kerr jk at ozlabs.org
Tue Sep 28 13:35:35 AEST 2021


Hi Lulu,

> When the same iso files are installed at the same time,
> the petitboot menu will only display the installed device first,
> and will not notify the user that the same iso file has been
> installed,
> so an alert is added to the system status log to remind the user that
> they have mounted the same iso file.
> 
> Signed-off-by: LuluTHSu <Lulu_Su at wistron.com>

If you're sending a second version of the patch, please include a short
description of the change. It's OK for now, but can be handy for future
revisions.

It looks like this version just has the discover_device_create()
removed, which is better. However, I still think it needs
simplification:

> @@ -184,6 +185,13 @@ static int udev_handle_block_add(struct pb_udev *udev, struct udev_device *dev,
>                 if (ddev) {
>                         pb_log("SKIP: %s UUID [%s] already present (as %s)\n",
>                                         name, uuid, ddev->device->id);
> +                       if (strncmp(name, ddev->device->id, strlen(ddev->device->id)) && (strlen(name) > strlen(ddev->device->id))) {
> +                               char temp[] = "sdx";
> +                               strncpy(temp, name, strlen(ddev->device->id));

This introduces a potential stack overflow - we may attempt to write
more data into 'temp' than it can store.

But still, I don't know why you're making the log message conditional
on not being a prefix of the other device. From your log in the previous
discussion, you end up with four devices with the same UUID; surely you
want to inform the user about all of them, rather than a subset? This
check makes a lot of assumptions about when a duplicate device is worth
logging or not.

In that case, this patch would be *really* simple: just unconditionally
call device_handler_status_info() with the log message. You probably
want to make it shorter so it can fit in the status line of a default
80-column petitboot UI.

Regards,


Jeremy



More information about the Petitboot mailing list