Skip to content

Commit 3a938c2

Browse files
aristeuaristeu
committed
efi/runtime-wrappers: Use type safe encapsulation of call arguments
JIRA: https://issues.redhat.com/browse/RHEL-22695 Tested: sanity commit c7c7bce Author: Ard Biesheuvel <ardb@kernel.org> Date: Sun Jul 2 15:15:18 2023 +0200 efi/runtime-wrappers: Use type safe encapsulation of call arguments The current code that marshalls the EFI runtime call arguments to hand them off to a async helper does so in a type unsafe and slightly messy manner - everything is cast to void* except for some integral types that are passed by reference and dereferenced on the receiver end. Let's clean this up a bit, and record the arguments of each runtime service invocation exactly as they are issued, in a manner that permits the compiler to check the types of the arguments at both ends. Signed-off-by: Ard Biesheuvel <ardb@kernel.org> Signed-off-by: Aristeu Rozanski <arozansk@redhat.com>
1 parent 2685881 commit 3a938c2

File tree

2 files changed

+138
-76
lines changed

2 files changed

+138
-76
lines changed

‎drivers/firmware/efi/runtime-wrappers.c‎

Lines changed: 130 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -42,20 +42,90 @@
4242
#define efi_call_virt(f, args...) \
4343
efi_call_virt_pointer(efi.runtime, f, args)
4444

45+
union efi_rts_args {
46+
struct {
47+
efi_time_t *time;
48+
efi_time_cap_t *capabilities;
49+
} GET_TIME;
50+
51+
struct {
52+
efi_time_t *time;
53+
} SET_TIME;
54+
55+
struct {
56+
efi_bool_t *enabled;
57+
efi_bool_t *pending;
58+
efi_time_t *time;
59+
} GET_WAKEUP_TIME;
60+
61+
struct {
62+
efi_bool_t enable;
63+
efi_time_t *time;
64+
} SET_WAKEUP_TIME;
65+
66+
struct {
67+
efi_char16_t *name;
68+
efi_guid_t *vendor;
69+
u32 *attr;
70+
unsigned long *data_size;
71+
void *data;
72+
} GET_VARIABLE;
73+
74+
struct {
75+
unsigned long *name_size;
76+
efi_char16_t *name;
77+
efi_guid_t *vendor;
78+
} GET_NEXT_VARIABLE;
79+
80+
struct {
81+
efi_char16_t *name;
82+
efi_guid_t *vendor;
83+
u32 attr;
84+
unsigned long data_size;
85+
void *data;
86+
} SET_VARIABLE;
87+
88+
struct {
89+
u32 attr;
90+
u64 *storage_space;
91+
u64 *remaining_space;
92+
u64 *max_variable_size;
93+
} QUERY_VARIABLE_INFO;
94+
95+
struct {
96+
u32 *high_count;
97+
} GET_NEXT_HIGH_MONO_COUNT;
98+
99+
struct {
100+
efi_capsule_header_t **capsules;
101+
unsigned long count;
102+
unsigned long sg_list;
103+
} UPDATE_CAPSULE;
104+
105+
struct {
106+
efi_capsule_header_t **capsules;
107+
unsigned long count;
108+
u64 *max_size;
109+
int *reset_type;
110+
} QUERY_CAPSULE_CAPS;
111+
};
112+
45113
struct efi_runtime_work efi_rts_work;
46114

47115
/*
48-
* efi_queue_work: Queue efi_runtime_service() and wait until it's done
49-
* @rts: efi_runtime_service() function identifier
50-
* @rts_arg<1-5>: efi_runtime_service() function arguments
116+
* efi_queue_work: Queue EFI runtime service call and wait for completion
117+
* @_rts: EFI runtime service function identifier
118+
* @_args: Arguments to pass to the EFI runtime service
51119
*
52120
* Accesses to efi_runtime_services() are serialized by a binary
53121
* semaphore (efi_runtime_lock) and caller waits until the work is
54122
* finished, hence _only_ one work is queued at a time and the caller
55123
* thread waits for completion.
56124
*/
57-
#define efi_queue_work(_rts, _arg1, _arg2, _arg3, _arg4, _arg5) \
125+
#define efi_queue_work(_rts, _args...) \
58126
({ \
127+
efi_rts_work.efi_rts_id = EFI_ ## _rts; \
128+
efi_rts_work.args = &(union efi_rts_args){ ._rts = { _args }}; \
59129
efi_rts_work.status = EFI_ABORTED; \
60130
\
61131
if (!efi_enabled(EFI_RUNTIME_SERVICES)) { \
@@ -66,12 +136,6 @@ struct efi_runtime_work efi_rts_work;
66136
\
67137
init_completion(&efi_rts_work.efi_rts_comp); \
68138
INIT_WORK(&efi_rts_work.work, efi_call_rts); \
69-
efi_rts_work.arg1 = _arg1; \
70-
efi_rts_work.arg2 = _arg2; \
71-
efi_rts_work.arg3 = _arg3; \
72-
efi_rts_work.arg4 = _arg4; \
73-
efi_rts_work.arg5 = _arg5; \
74-
efi_rts_work.efi_rts_id = _rts; \
75139
\
76140
/* \
77141
* queue_work() returns 0 if work was already on queue, \
@@ -168,73 +232,78 @@ extern struct semaphore __efi_uv_runtime_lock __alias(efi_runtime_lock);
168232
/*
169233
* Calls the appropriate efi_runtime_service() with the appropriate
170234
* arguments.
171-
*
172-
* Semantics followed by efi_call_rts() to understand efi_runtime_work:
173-
* 1. If argument was a pointer, recast it from void pointer to original
174-
* pointer type.
175-
* 2. If argument was a value, recast it from void pointer to original
176-
* pointer type and dereference it.
177235
*/
178236
static void efi_call_rts(struct work_struct *work)
179237
{
180-
void *arg1, *arg2, *arg3, *arg4, *arg5;
238+
const union efi_rts_args *args = efi_rts_work.args;
181239
efi_status_t status = EFI_NOT_FOUND;
182240

183-
arg1 = efi_rts_work.arg1;
184-
arg2 = efi_rts_work.arg2;
185-
arg3 = efi_rts_work.arg3;
186-
arg4 = efi_rts_work.arg4;
187-
arg5 = efi_rts_work.arg5;
188-
189241
switch (efi_rts_work.efi_rts_id) {
190242
case EFI_GET_TIME:
191-
status = efi_call_virt(get_time, (efi_time_t *)arg1,
192-
(efi_time_cap_t *)arg2);
243+
status = efi_call_virt(get_time,
244+
args->GET_TIME.time,
245+
args->GET_TIME.capabilities);
193246
break;
194247
case EFI_SET_TIME:
195-
status = efi_call_virt(set_time, (efi_time_t *)arg1);
248+
status = efi_call_virt(set_time,
249+
args->SET_TIME.time);
196250
break;
197251
case EFI_GET_WAKEUP_TIME:
198-
status = efi_call_virt(get_wakeup_time, (efi_bool_t *)arg1,
199-
(efi_bool_t *)arg2, (efi_time_t *)arg3);
252+
status = efi_call_virt(get_wakeup_time,
253+
args->GET_WAKEUP_TIME.enabled,
254+
args->GET_WAKEUP_TIME.pending,
255+
args->GET_WAKEUP_TIME.time);
200256
break;
201257
case EFI_SET_WAKEUP_TIME:
202-
status = efi_call_virt(set_wakeup_time, *(efi_bool_t *)arg1,
203-
(efi_time_t *)arg2);
258+
status = efi_call_virt(set_wakeup_time,
259+
args->SET_WAKEUP_TIME.enable,
260+
args->SET_WAKEUP_TIME.time);
204261
break;
205262
case EFI_GET_VARIABLE:
206-
status = efi_call_virt(get_variable, (efi_char16_t *)arg1,
207-
(efi_guid_t *)arg2, (u32 *)arg3,
208-
(unsigned long *)arg4, (void *)arg5);
263+
status = efi_call_virt(get_variable,
264+
args->GET_VARIABLE.name,
265+
args->GET_VARIABLE.vendor,
266+
args->GET_VARIABLE.attr,
267+
args->GET_VARIABLE.data_size,
268+
args->GET_VARIABLE.data);
209269
break;
210270
case EFI_GET_NEXT_VARIABLE:
211-
status = efi_call_virt(get_next_variable, (unsigned long *)arg1,
212-
(efi_char16_t *)arg2,
213-
(efi_guid_t *)arg3);
271+
status = efi_call_virt(get_next_variable,
272+
args->GET_NEXT_VARIABLE.name_size,
273+
args->GET_NEXT_VARIABLE.name,
274+
args->GET_NEXT_VARIABLE.vendor);
214275
break;
215276
case EFI_SET_VARIABLE:
216-
status = efi_call_virt(set_variable, (efi_char16_t *)arg1,
217-
(efi_guid_t *)arg2, *(u32 *)arg3,
218-
*(unsigned long *)arg4, (void *)arg5);
277+
status = efi_call_virt(set_variable,
278+
args->SET_VARIABLE.name,
279+
args->SET_VARIABLE.vendor,
280+
args->SET_VARIABLE.attr,
281+
args->SET_VARIABLE.data_size,
282+
args->SET_VARIABLE.data);
219283
break;
220284
case EFI_QUERY_VARIABLE_INFO:
221-
status = efi_call_virt(query_variable_info, *(u32 *)arg1,
222-
(u64 *)arg2, (u64 *)arg3, (u64 *)arg4);
285+
status = efi_call_virt(query_variable_info,
286+
args->QUERY_VARIABLE_INFO.attr,
287+
args->QUERY_VARIABLE_INFO.storage_space,
288+
args->QUERY_VARIABLE_INFO.remaining_space,
289+
args->QUERY_VARIABLE_INFO.max_variable_size);
223290
break;
224291
case EFI_GET_NEXT_HIGH_MONO_COUNT:
225-
status = efi_call_virt(get_next_high_mono_count, (u32 *)arg1);
292+
status = efi_call_virt(get_next_high_mono_count,
293+
args->GET_NEXT_HIGH_MONO_COUNT.high_count);
226294
break;
227295
case EFI_UPDATE_CAPSULE:
228296
status = efi_call_virt(update_capsule,
229-
(efi_capsule_header_t **)arg1,
230-
*(unsigned long *)arg2,
231-
*(unsigned long *)arg3);
297+
args->UPDATE_CAPSULE.capsules,
298+
args->UPDATE_CAPSULE.count,
299+
args->UPDATE_CAPSULE.sg_list);
232300
break;
233301
case EFI_QUERY_CAPSULE_CAPS:
234302
status = efi_call_virt(query_capsule_caps,
235-
(efi_capsule_header_t **)arg1,
236-
*(unsigned long *)arg2, (u64 *)arg3,
237-
(int *)arg4);
303+
args->QUERY_CAPSULE_CAPS.capsules,
304+
args->QUERY_CAPSULE_CAPS.count,
305+
args->QUERY_CAPSULE_CAPS.max_size,
306+
args->QUERY_CAPSULE_CAPS.reset_type);
238307
break;
239308
default:
240309
/*
@@ -254,7 +323,7 @@ static efi_status_t virt_efi_get_time(efi_time_t *tm, efi_time_cap_t *tc)
254323

255324
if (down_interruptible(&efi_runtime_lock))
256325
return EFI_ABORTED;
257-
status = efi_queue_work(EFI_GET_TIME, tm, tc, NULL, NULL, NULL);
326+
status = efi_queue_work(GET_TIME, tm, tc);
258327
up(&efi_runtime_lock);
259328
return status;
260329
}
@@ -265,7 +334,7 @@ static efi_status_t virt_efi_set_time(efi_time_t *tm)
265334

266335
if (down_interruptible(&efi_runtime_lock))
267336
return EFI_ABORTED;
268-
status = efi_queue_work(EFI_SET_TIME, tm, NULL, NULL, NULL, NULL);
337+
status = efi_queue_work(SET_TIME, tm);
269338
up(&efi_runtime_lock);
270339
return status;
271340
}
@@ -278,8 +347,7 @@ static efi_status_t virt_efi_get_wakeup_time(efi_bool_t *enabled,
278347

279348
if (down_interruptible(&efi_runtime_lock))
280349
return EFI_ABORTED;
281-
status = efi_queue_work(EFI_GET_WAKEUP_TIME, enabled, pending, tm, NULL,
282-
NULL);
350+
status = efi_queue_work(GET_WAKEUP_TIME, enabled, pending, tm);
283351
up(&efi_runtime_lock);
284352
return status;
285353
}
@@ -290,8 +358,7 @@ static efi_status_t virt_efi_set_wakeup_time(efi_bool_t enabled, efi_time_t *tm)
290358

291359
if (down_interruptible(&efi_runtime_lock))
292360
return EFI_ABORTED;
293-
status = efi_queue_work(EFI_SET_WAKEUP_TIME, &enabled, tm, NULL, NULL,
294-
NULL);
361+
status = efi_queue_work(SET_WAKEUP_TIME, enabled, tm);
295362
up(&efi_runtime_lock);
296363
return status;
297364
}
@@ -306,7 +373,7 @@ static efi_status_t virt_efi_get_variable(efi_char16_t *name,
306373

307374
if (down_interruptible(&efi_runtime_lock))
308375
return EFI_ABORTED;
309-
status = efi_queue_work(EFI_GET_VARIABLE, name, vendor, attr, data_size,
376+
status = efi_queue_work(GET_VARIABLE, name, vendor, attr, data_size,
310377
data);
311378
up(&efi_runtime_lock);
312379
return status;
@@ -320,8 +387,7 @@ static efi_status_t virt_efi_get_next_variable(unsigned long *name_size,
320387

321388
if (down_interruptible(&efi_runtime_lock))
322389
return EFI_ABORTED;
323-
status = efi_queue_work(EFI_GET_NEXT_VARIABLE, name_size, name, vendor,
324-
NULL, NULL);
390+
status = efi_queue_work(GET_NEXT_VARIABLE, name_size, name, vendor);
325391
up(&efi_runtime_lock);
326392
return status;
327393
}
@@ -336,7 +402,7 @@ static efi_status_t virt_efi_set_variable(efi_char16_t *name,
336402

337403
if (down_interruptible(&efi_runtime_lock))
338404
return EFI_ABORTED;
339-
status = efi_queue_work(EFI_SET_VARIABLE, name, vendor, &attr, &data_size,
405+
status = efi_queue_work(SET_VARIABLE, name, vendor, attr, data_size,
340406
data);
341407
up(&efi_runtime_lock);
342408
return status;
@@ -371,8 +437,8 @@ static efi_status_t virt_efi_query_variable_info(u32 attr,
371437

372438
if (down_interruptible(&efi_runtime_lock))
373439
return EFI_ABORTED;
374-
status = efi_queue_work(EFI_QUERY_VARIABLE_INFO, &attr, storage_space,
375-
remaining_space, max_variable_size, NULL);
440+
status = efi_queue_work(QUERY_VARIABLE_INFO, attr, storage_space,
441+
remaining_space, max_variable_size);
376442
up(&efi_runtime_lock);
377443
return status;
378444
}
@@ -403,8 +469,7 @@ static efi_status_t virt_efi_get_next_high_mono_count(u32 *count)
403469

404470
if (down_interruptible(&efi_runtime_lock))
405471
return EFI_ABORTED;
406-
status = efi_queue_work(EFI_GET_NEXT_HIGH_MONO_COUNT, count, NULL, NULL,
407-
NULL, NULL);
472+
status = efi_queue_work(GET_NEXT_HIGH_MONO_COUNT, count);
408473
up(&efi_runtime_lock);
409474
return status;
410475
}
@@ -440,8 +505,7 @@ static efi_status_t virt_efi_update_capsule(efi_capsule_header_t **capsules,
440505

441506
if (down_interruptible(&efi_runtime_lock))
442507
return EFI_ABORTED;
443-
status = efi_queue_work(EFI_UPDATE_CAPSULE, capsules, &count, &sg_list,
444-
NULL, NULL);
508+
status = efi_queue_work(UPDATE_CAPSULE, capsules, count, sg_list);
445509
up(&efi_runtime_lock);
446510
return status;
447511
}
@@ -458,8 +522,8 @@ static efi_status_t virt_efi_query_capsule_caps(efi_capsule_header_t **capsules,
458522

459523
if (down_interruptible(&efi_runtime_lock))
460524
return EFI_ABORTED;
461-
status = efi_queue_work(EFI_QUERY_CAPSULE_CAPS, capsules, &count,
462-
max_size, reset_type, NULL);
525+
status = efi_queue_work(QUERY_CAPSULE_CAPS, capsules, count,
526+
max_size, reset_type);
463527
up(&efi_runtime_lock);
464528
return status;
465529
}

‎include/linux/efi.h‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1295,23 +1295,21 @@ enum efi_rts_ids {
12951295
EFI_QUERY_CAPSULE_CAPS,
12961296
};
12971297

1298+
union efi_rts_args;
1299+
12981300
/*
12991301
* efi_runtime_work: Details of EFI Runtime Service work
1300-
* @arg<1-5>: EFI Runtime Service function arguments
1302+
* @args: Pointer to union describing the arguments
13011303
* @status: Status of executing EFI Runtime Service
13021304
* @efi_rts_id: EFI Runtime Service function identifier
13031305
* @efi_rts_comp: Struct used for handling completions
13041306
*/
13051307
struct efi_runtime_work {
1306-
void *arg1;
1307-
void *arg2;
1308-
void *arg3;
1309-
void *arg4;
1310-
void *arg5;
1311-
efi_status_t status;
1312-
struct work_struct work;
1313-
enum efi_rts_ids efi_rts_id;
1314-
struct completion efi_rts_comp;
1308+
union efi_rts_args *args;
1309+
efi_status_t status;
1310+
struct work_struct work;
1311+
enum efi_rts_ids efi_rts_id;
1312+
struct completion efi_rts_comp;
13151313
};
13161314

13171315
extern struct efi_runtime_work efi_rts_work;

0 commit comments

Comments
 (0)