Re: [PATCH v9 5/5] eficonfig: add "Show Signature Database" menu entry

[Heinrich Schuchardt]

On 11/17/22 22:49, Ilias Apalodimas wrote:

Hi Kojima-san

+/**
+ * prepare_signature_list_menu() - create the signature list menu entry
+ *
+ * @efimenu:   pointer to the efimenu structure
+ * @varname:   pointer to the variable name
+ * @db:                pointer to the variable raw data
+ * @db_size:   variable data size
+ * @func:      callback of each entry
+ * Return:     status code
+ */
+static efi_status_t prepare_signature_list_menu(struct efimenu *efi_menu, void 
*varname,
+                                               void *db, efi_uintn_t db_size,
+                                               eficonfig_entry_func func)
+{
+       u32 num = 0;
+       efi_uintn_t size;
+       struct eficonfig_sig_data *sg;
+       struct efi_signature_list *esl;
+       struct efi_signature_data *esd;
+       efi_status_t ret = EFI_SUCCESS;
+
+       INIT_LIST_HEAD(&efi_menu->list);
+
+       esl = db;
+       size = db_size;
+       while (size > 0) {
+               u32 remain;
+
+               esd = (struct efi_signature_data *)((u8 *)esl +
+                                                   (sizeof(struct 
efi_signature_list) +
+                                                   
esl->signature_header_size));
+               remain = esl->signature_list_size - sizeof(struct 
efi_signature_list) -
+                        esl->signature_header_size;
+               for (; remain > 0; remain -= esl->signature_size) {
+                       char buf[40];

This is going to hold a GUID right? Isn't buf[37] enough?

+                       char *title;
+
+                       if (num >= EFICONFIG_ENTRY_NUM_MAX - 1) {
+                               ret = EFI_OUT_OF_RESOURCES;
+                               goto out;
+                       }
+
+                       sg = calloc(1, sizeof(struct eficonfig_sig_data));
+                       if (!sg) {
+                               ret = EFI_OUT_OF_RESOURCES;
+                               goto err;
+                       }
+
+                       snprintf(buf, sizeof(buf), "%pUL", 
&esd->signature_owner);
+                       title = calloc(1, (strlen(buf) + 1));

Since the length will always be static, just use sizeof(buf)

You can use strdup() instead of strlen + calloc + strlcpy.

Best regards

Heinrich

+                       if (!title) {
+                               free(sg);
+                               ret = EFI_OUT_OF_RESOURCES;
+                               goto err;
+                       }
+                       strlcpy(title, buf, strlen(buf) + 1);
+
+                       sg->esl = esl;
+                       sg->esd = esd;
+                       sg->varname = varname;
+                       ret = eficonfig_append_menu_entry(efi_menu, title, 
func, sg);
+                       if (ret != EFI_SUCCESS) {
+                               free(sg);
+                               free(title);
+                               goto err;
+                       }
+                       esd = (struct efi_signature_data *)((u8 *)esd + 
esl->signature_size);
+                       num++;
+               }
+
+               size -= esl->signature_list_size;
+               esl = (struct efi_signature_list *)((u8 *)esl + 
esl->signature_list_size);
+       }
+out:
+       ret = eficonfig_append_quit_entry(efi_menu);
+err:
+       return ret;

With those
Acked-by: Ilias Apalodimas <ilias.apalodi...@linaro.org>