Currently the keyctl helper makes silent assumption that either all expected arguments are being provided or otherwise it's somewhat safe to access beyond the actual va arg list. With AAPCS64-cap this will no longer slide through, as reading more arguments than provided will trigger capability bound fault. Empty va_list is another issue there (see KEYCTL_SESSION_TO_PARENT).
Introduce dedicated macro to handle the variadic-ness instead, making sure that at any point all arguments are being provided, whether explicitly or through default values.
Additionally move the keyctl_join_session_keyring opening brace to the beginning of next line to align with coding style.
Reported-by: Kevin Brodsky kevin.brodsky@arm.com Suggested-by: Kevin Brodsky kevin.brodsky@arm.com Signed-off-by: Beata Michalska beata.michalska@arm.com --- changes available at: https://git.morello-project.org/Bea/morello-linux-ltp/-/tree/morello/keyctl_...
v3: - Fixed alignment - Addded info in the commit msg on unrelevant change - Dropped a section on mapping command to nr of args v2: - define keyctl as macro so that the kectl users can remain unaware of the changes (thanks to @Tudor) include/lapi/keyctl.h | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/include/lapi/keyctl.h b/include/lapi/keyctl.h index 9c847a429..e1af4d302 100644 --- a/include/lapi/keyctl.h +++ b/include/lapi/keyctl.h @@ -39,22 +39,20 @@ static inline key_serial_t request_key(const char *type, type, description, callout_info, destringid); }
-static inline long keyctl(int cmd, ...) -{ - va_list va; - uintptr_t arg2, arg3, arg4, arg5; +#define __keyctl(cmd, arg2, arg3, arg4, arg5, ...) \ +({ \ + long result; \ + result = tst_syscall(__NR_keyctl, (cmd), \ + (arg2), (arg3), (arg4), (arg5)); \ + result; \ +})
- va_start(va, cmd); - arg2 = va_arg(va, uintptr_t); - arg3 = va_arg(va, uintptr_t); - arg4 = va_arg(va, uintptr_t); - arg5 = va_arg(va, uintptr_t); - va_end(va); +#define keyctl(cmd, ...) \ + __keyctl((cmd), ##__VA_ARGS__, 0, 0, 0, 0)
- return tst_syscall(__NR_keyctl, cmd, arg2, arg3, arg4, arg5); -}
-static inline key_serial_t keyctl_join_session_keyring(const char *name) { +static inline key_serial_t keyctl_join_session_keyring(const char *name) +{ return keyctl(KEYCTL_JOIN_SESSION_KEYRING, name); }
On 05-12-2022 15:31, Beata Michalska wrote:
Currently the keyctl helper makes silent assumption that either all expected arguments are being provided or otherwise it's somewhat safe to access beyond the actual va arg list. With AAPCS64-cap this will no longer slide through, as reading more arguments than provided will trigger capability bound fault. Empty va_list is another issue there (see KEYCTL_SESSION_TO_PARENT).
Introduce dedicated macro to handle the variadic-ness instead, making sure that at any point all arguments are being provided, whether explicitly or through default values.
Additionally move the keyctl_join_session_keyring opening brace to the beginning of next line to align with coding style.
Reported-by: Kevin Brodsky kevin.brodsky@arm.com Suggested-by: Kevin Brodsky kevin.brodsky@arm.com Signed-off-by: Beata Michalska beata.michalska@arm.com
LGTM!
Thanks, Tudor
changes available at: https://git.morello-project.org/Bea/morello-linux-ltp/-/tree/morello/keyctl_...
v3:
- Fixed alignment
- Addded info in the commit msg on unrelevant change
- Dropped a section on mapping command to nr of args
v2:
- define keyctl as macro so that the kectl users can remain unaware of the changes (thanks to @Tudor)
include/lapi/keyctl.h | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/include/lapi/keyctl.h b/include/lapi/keyctl.h index 9c847a429..e1af4d302 100644 --- a/include/lapi/keyctl.h +++ b/include/lapi/keyctl.h @@ -39,22 +39,20 @@ static inline key_serial_t request_key(const char *type, type, description, callout_info, destringid); } -static inline long keyctl(int cmd, ...) -{
- va_list va;
- uintptr_t arg2, arg3, arg4, arg5;
+#define __keyctl(cmd, arg2, arg3, arg4, arg5, ...) \ +({ \
- long result; \
- result = tst_syscall(__NR_keyctl, (cmd), \
(arg2), (arg3), (arg4), (arg5)); \- result; \
+})
- va_start(va, cmd);
- arg2 = va_arg(va, uintptr_t);
- arg3 = va_arg(va, uintptr_t);
- arg4 = va_arg(va, uintptr_t);
- arg5 = va_arg(va, uintptr_t);
- va_end(va);
+#define keyctl(cmd, ...) \
- __keyctl((cmd), ##__VA_ARGS__, 0, 0, 0, 0)
- return tst_syscall(__NR_keyctl, cmd, arg2, arg3, arg4, arg5);
-} -static inline key_serial_t keyctl_join_session_keyring(const char *name) { +static inline key_serial_t keyctl_join_session_keyring(const char *name) +{ return keyctl(KEYCTL_JOIN_SESSION_KEYRING, name); }
On 05/12/2022 16:50, Tudor Cretu wrote:
On 05-12-2022 15:31, Beata Michalska wrote:
Currently the keyctl helper makes silent assumption that either all expected arguments are being provided or otherwise it's somewhat safe to access beyond the actual va arg list. With AAPCS64-cap this will no longer slide through, as reading more arguments than provided will trigger capability bound fault. Empty va_list is another issue there (see KEYCTL_SESSION_TO_PARENT).
Introduce dedicated macro to handle the variadic-ness instead, making sure that at any point all arguments are being provided, whether explicitly or through default values.
Additionally move the keyctl_join_session_keyring opening brace to the beginning of next line to align with coding style.
Reported-by: Kevin Brodsky kevin.brodsky@arm.com Suggested-by: Kevin Brodsky kevin.brodsky@arm.com Signed-off-by: Beata Michalska beata.michalska@arm.com
LGTM!
+1 :)
Kevin
Thanks, Tudor
changes available at: https://git.morello-project.org/Bea/morello-linux-ltp/-/tree/morello/keyctl_...
v3: - Fixed alignment - Addded info in the commit msg on unrelevant change - Dropped a section on mapping command to nr of args v2: - define keyctl as macro so that the kectl users can remain unaware of the changes (thanks to @Tudor) include/lapi/keyctl.h | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/include/lapi/keyctl.h b/include/lapi/keyctl.h index 9c847a429..e1af4d302 100644 --- a/include/lapi/keyctl.h +++ b/include/lapi/keyctl.h @@ -39,22 +39,20 @@ static inline key_serial_t request_key(const char *type, type, description, callout_info, destringid); } -static inline long keyctl(int cmd, ...) -{ - va_list va; - uintptr_t arg2, arg3, arg4, arg5; +#define __keyctl(cmd, arg2, arg3, arg4, arg5, ...) \ +({ \ + long result; \ + result = tst_syscall(__NR_keyctl, (cmd), \ + (arg2), (arg3), (arg4), (arg5)); \ + result; \ +}) - va_start(va, cmd); - arg2 = va_arg(va, uintptr_t); - arg3 = va_arg(va, uintptr_t); - arg4 = va_arg(va, uintptr_t); - arg5 = va_arg(va, uintptr_t); - va_end(va); +#define keyctl(cmd, ...) \ + __keyctl((cmd), ##__VA_ARGS__, 0, 0, 0, 0) - return tst_syscall(__NR_keyctl, cmd, arg2, arg3, arg4, arg5); -} -static inline key_serial_t keyctl_join_session_keyring(const char *name) { +static inline key_serial_t keyctl_join_session_keyring(const char *name) +{ return keyctl(KEYCTL_JOIN_SESSION_KEYRING, name); }
linux-morello-ltp mailing list -- linux-morello-ltp@op-lists.linaro.org To unsubscribe send an email to linux-morello-ltp-leave@op-lists.linaro.org
linux-morello-ltp@op-lists.linaro.org
-
Beata Michalska -
Kevin Brodsky -
Tudor Cretu