On 3 May 2016 at 16:50, Matt Fleming <matt@xxxxxxxxxxxxxxxxxxx> wrote: > On Mon, 02 May, at 09:25:00PM, Dan Carpenter wrote: >> Hello Matt Fleming, >> >> The patch f0133f3c5b8b: "efi: Add 'capsule' update support" from Apr >> 25, 2016, leads to the following static checker warning: >> >> drivers/firmware/efi/capsule.c:109 efi_capsule_supported() >> warn: did you mean to pass the address of 'capsule' >> >> drivers/firmware/efi/capsule.c >> 91 int efi_capsule_supported(efi_guid_t guid, u32 flags, size_t size, int *reset) >> 92 { >> 93 efi_capsule_header_t *capsule; >> 94 efi_status_t status; >> 95 u64 max_size; >> 96 int rv = 0; >> 97 >> 98 if (flags & ~EFI_CAPSULE_SUPPORTED_FLAG_MASK) >> 99 return -EINVAL; >> 100 >> 101 capsule = kmalloc(sizeof(*capsule), GFP_KERNEL); >> 102 if (!capsule) >> 103 return -ENOMEM; >> 104 >> 105 capsule->headersize = capsule->imagesize = sizeof(*capsule); >> 106 memcpy(&capsule->guid, &guid, sizeof(efi_guid_t)); >> 107 capsule->flags = flags; >> 108 >> 109 status = efi.query_capsule_caps(&capsule, 1, &max_size, reset); >> ^^^^^^^^ >> If we modify capsule inside this function call then at the end of the >> function we aren't freeing the original pointer that we allocated. >> >> 110 if (status != EFI_SUCCESS) { >> 111 rv = efi_status_to_err(status); >> 112 goto out; >> 113 } >> 114 >> 115 if (size > max_size) >> 116 rv = -ENOSPC; >> 117 out: >> 118 kfree(capsule); >> 119 return rv; >> 120 } > > We should be fine here, the firmware should not modify the argument > that we pass since we're simply querying whether or not the capsule is > supported. > > Is there a cleanup that you'd suggest making to silence the static > checker warning? Well, I suppose we could simply allocate the pointer array and the single member statically, i.e., efi_capsule_header_t capsule1; efi_capsule_header_t *capsule[] = { &capsule1 }; That way, we can get rid of the heap allocation entirely, and we take the address of the array, i.e., without the & -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html