On 19.11.24 11:34, Ilias Apalodimas wrote:
On Tue, 19 Nov 2024 at 11:50, Heinrich Schuchardt
<[email protected]> wrote:
On 18.11.24 22:08, Adriano Cordova wrote:
Add efi_dp_from_ipv4 to form a device path from an ipv4 address.
Signed-off-by: Adriano Cordova <[email protected]>
---
Changes in v4:
- Fixed memcpy mistake
Changes in v3:
- Removed some unnecessary void* casts.
- Changed sizeof(struct efi_device_path_ipv4) to sizeof(*ipv4dp)
in efi_dp_from_ipv4.
lib/efi_loader/efi_device_path.c | 38 ++++++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
index ee387e1dfd..cdeea4791f 100644
--- a/lib/efi_loader/efi_device_path.c
+++ b/lib/efi_loader/efi_device_path.c
@@ -974,6 +974,44 @@ struct efi_device_path __maybe_unused
*efi_dp_from_eth(void)
return start;
}
A function description is missing.
+struct efi_device_path *efi_dp_from_ipv4(struct efi_ipv4_address *ip,
+ struct efi_ipv4_address *mask,
+ struct efi_ipv4_address *srv)
+{
+ struct efi_device_path *dp1, *dp2;
+ efi_uintn_t ipv4dp_len;
+ struct efi_device_path_ipv4 *ipv4dp;
+ char *pos;
+
+ ipv4dp_len = sizeof(*ipv4dp);
+ ipv4dp = efi_alloc(ipv4dp_len + sizeof(END));
ipv4dp is a small structure and is freed below.
Allocating it on the stack would reduce the code size.
+ if (!ipv4dp) {
+ log_err("Out of memory\n");
Writing messages inside a protocol implementation should be avoided.
+ return NULL;
+ }
+
+ ipv4dp->dp.type = DEVICE_PATH_TYPE_MESSAGING_DEVICE;
+ ipv4dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_IPV4;
+ ipv4dp->dp.length = ipv4dp_len;
+ ipv4dp->protocol = 6;
+ if (ip)
+ memcpy(&ipv4dp->local_ip_address, ip, sizeof(*ip));
+ if (mask)
+ memcpy(&ipv4dp->subnet_mask, mask, sizeof(*mask));
+ if (srv)
+ memcpy(&ipv4dp->remote_ip_address, srv, sizeof(*srv));
+ pos = (void *)(uintptr_t)ipv4dp + ipv4dp_len;
Here you
1 - convert ipv4dp to uintptr_t
2 - convert the uintptr_t value to void *
3 - add an offset to the void * value
In the C standard adding an offset to a void * value results in
undefined behavior. Unfortunately U-Boot code uses it a lot.
It's a known support gcc extension that treats void* similar to char*.
Do you think it's worth changing all of the call sites we have?
No, I don't request this. But as pos is already char*, we should not use
void* here.
Best regards
Heinrich
pos = (char *)ipv4dp + ipv4dp_len;
is all it takes to have a defined behavior here.
Best regards
Heinrich
+ memcpy(pos, &END, sizeof(END));
+
+ dp1 = efi_dp_from_eth();
+ dp2 = efi_dp_concat(dp1, (const struct efi_device_path *)ipv4dp, 0);
+
+ efi_free_pool(ipv4dp);
+ efi_free_pool(dp1);
+
+ return dp2;
+}
+
/* Construct a device-path for memory-mapped image */
struct efi_device_path *efi_dp_from_mem(uint32_t memory_type,
uint64_t start_address,
