Change security_capget LSM hook's target parameter to constant and split
cap_capget decalration line to multiple lines.
Khadija Kamran (2):
lsm: change 'target' parameter to 'const' in security_capget LSM hook
lsm: split cap_capget() declaration to multiple lines
include/linux/lsm_hook_defs.h | 2 +-
include/linux/security.h | 7 ++++---
kernel/capability.c | 2 +-
security/apparmor/lsm.c | 2 +-
security/commoncap.c | 2 +-
security/security.c | 2 +-
security/selinux/hooks.c | 2 +-
7 files changed, 10 insertions(+), 9 deletions(-)
--
2.34.1
cap_capget(...) LSM hook declaration exceeds the 80 characters per line
limit. Split the function declaration to multple lines to decrease the
line length.
Signed-off-by: Khadija Kamran <[email protected]>
---
include/linux/security.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/linux/security.h b/include/linux/security.h
index 8b7d0b2ec1a4..fef65d0e522d 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -145,7 +145,8 @@ extern int cap_capable(const struct cred *cred, struct user_namespace *ns,
extern int cap_settime(const struct timespec64 *ts, const struct timezone *tz);
extern int cap_ptrace_access_check(struct task_struct *child, unsigned int mode);
extern int cap_ptrace_traceme(struct task_struct *parent);
-extern int cap_capget(const struct task_struct *target, kernel_cap_t *effective, kernel_cap_t *inheritable, kernel_cap_t *permitted);
+extern int cap_capget(const struct task_struct *target, kernel_cap_t *effective,
+ kernel_cap_t *inheritable, kernel_cap_t *permitted);
extern int cap_capset(struct cred *new, const struct cred *old,
const kernel_cap_t *effective,
const kernel_cap_t *inheritable,
--
2.34.1
Three LSMs register the implementations for the "capget" hook: AppArmor,
SELinux, and the normal capability code. Looking at the function
implementations we may observe that the first parameter "target" is not
changing.
Mark the first argument "target" of LSM hook security_capget(...) as
"const" since it will not be changing in the LSM hook.
Signed-off-by: Khadija Kamran <[email protected]>
---
include/linux/lsm_hook_defs.h | 2 +-
include/linux/security.h | 6 +++---
kernel/capability.c | 2 +-
security/apparmor/lsm.c | 2 +-
security/commoncap.c | 2 +-
security/security.c | 2 +-
security/selinux/hooks.c | 2 +-
7 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 6bb55e61e8e8..fd3844e11077 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -36,7 +36,7 @@ LSM_HOOK(int, 0, binder_transfer_file, const struct cred *from,
LSM_HOOK(int, 0, ptrace_access_check, struct task_struct *child,
unsigned int mode)
LSM_HOOK(int, 0, ptrace_traceme, struct task_struct *parent)
-LSM_HOOK(int, 0, capget, struct task_struct *target, kernel_cap_t *effective,
+LSM_HOOK(int, 0, capget, const struct task_struct *target, kernel_cap_t *effective,
kernel_cap_t *inheritable, kernel_cap_t *permitted)
LSM_HOOK(int, 0, capset, struct cred *new, const struct cred *old,
const kernel_cap_t *effective, const kernel_cap_t *inheritable,
diff --git a/include/linux/security.h b/include/linux/security.h
index e2734e9e44d5..8b7d0b2ec1a4 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -145,7 +145,7 @@ extern int cap_capable(const struct cred *cred, struct user_namespace *ns,
extern int cap_settime(const struct timespec64 *ts, const struct timezone *tz);
extern int cap_ptrace_access_check(struct task_struct *child, unsigned int mode);
extern int cap_ptrace_traceme(struct task_struct *parent);
-extern int cap_capget(struct task_struct *target, kernel_cap_t *effective, kernel_cap_t *inheritable, kernel_cap_t *permitted);
+extern int cap_capget(const struct task_struct *target, kernel_cap_t *effective, kernel_cap_t *inheritable, kernel_cap_t *permitted);
extern int cap_capset(struct cred *new, const struct cred *old,
const kernel_cap_t *effective,
const kernel_cap_t *inheritable,
@@ -271,7 +271,7 @@ int security_binder_transfer_file(const struct cred *from,
const struct cred *to, struct file *file);
int security_ptrace_access_check(struct task_struct *child, unsigned int mode);
int security_ptrace_traceme(struct task_struct *parent);
-int security_capget(struct task_struct *target,
+int security_capget(const struct task_struct *target,
kernel_cap_t *effective,
kernel_cap_t *inheritable,
kernel_cap_t *permitted);
@@ -553,7 +553,7 @@ static inline int security_ptrace_traceme(struct task_struct *parent)
return cap_ptrace_traceme(parent);
}
-static inline int security_capget(struct task_struct *target,
+static inline int security_capget(const struct task_struct *target,
kernel_cap_t *effective,
kernel_cap_t *inheritable,
kernel_cap_t *permitted)
diff --git a/kernel/capability.c b/kernel/capability.c
index 3e058f41df32..67bdee3414dd 100644
--- a/kernel/capability.c
+++ b/kernel/capability.c
@@ -112,7 +112,7 @@ static inline int cap_get_target_pid(pid_t pid, kernel_cap_t *pEp,
int ret;
if (pid && (pid != task_pid_vnr(current))) {
- struct task_struct *target;
+ const struct task_struct *target;
rcu_read_lock();
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index f431251ffb91..12dd96c3b2f0 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -144,7 +144,7 @@ static int apparmor_ptrace_traceme(struct task_struct *parent)
}
/* Derived from security/commoncap.c:cap_capget */
-static int apparmor_capget(struct task_struct *target, kernel_cap_t *effective,
+static int apparmor_capget(const struct task_struct *target, kernel_cap_t *effective,
kernel_cap_t *inheritable, kernel_cap_t *permitted)
{
struct aa_label *label;
diff --git a/security/commoncap.c b/security/commoncap.c
index 0b3fc2f3afe7..5fd64d3e5bfd 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -197,7 +197,7 @@ int cap_ptrace_traceme(struct task_struct *parent)
* This function retrieves the capabilities of the nominated task and returns
* them to the caller.
*/
-int cap_capget(struct task_struct *target, kernel_cap_t *effective,
+int cap_capget(const struct task_struct *target, kernel_cap_t *effective,
kernel_cap_t *inheritable, kernel_cap_t *permitted)
{
const struct cred *cred;
diff --git a/security/security.c b/security/security.c
index d5ff7ff45b77..fb2d93b481f1 100644
--- a/security/security.c
+++ b/security/security.c
@@ -893,7 +893,7 @@ int security_ptrace_traceme(struct task_struct *parent)
*
* Return: Returns 0 if the capability sets were successfully obtained.
*/
-int security_capget(struct task_struct *target,
+int security_capget(const struct task_struct *target,
kernel_cap_t *effective,
kernel_cap_t *inheritable,
kernel_cap_t *permitted)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 79b4890e9936..ff42d49f1b41 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2056,7 +2056,7 @@ static int selinux_ptrace_traceme(struct task_struct *parent)
SECCLASS_PROCESS, PROCESS__PTRACE, NULL);
}
-static int selinux_capget(struct task_struct *target, kernel_cap_t *effective,
+static int selinux_capget(const struct task_struct *target, kernel_cap_t *effective,
kernel_cap_t *inheritable, kernel_cap_t *permitted)
{
return avc_has_perm(current_sid(), task_sid_obj(target),
--
2.34.1
On Mon, Jul 31, 2023 at 12:07:31PM +0500, Khadija Kamran wrote:
> Three LSMs register the implementations for the "capget" hook: AppArmor,
> SELinux, and the normal capability code. Looking at the function
> implementations we may observe that the first parameter "target" is not
> changing.
>
> Mark the first argument "target" of LSM hook security_capget(...) as
> "const" since it will not be changing in the LSM hook.
The commit message may be simplified, from this:
[PATCH 1/2] lsm: change 'target' parameter to 'const' in security_capget LSM hook
to something like this:
[PATCH 1/2] lsm: Constify the target parameter in security_capget()
"Constify" is the commonly used language in git logs.
See git log --pretty=oneline --abbrev-commit | grep Constify
Alison
--snip --
>
On Mon, Jul 31, 2023 at 1:02 PM Alison Schofield
<[email protected]> wrote:
> On Mon, Jul 31, 2023 at 12:07:31PM +0500, Khadija Kamran wrote:
> > Three LSMs register the implementations for the "capget" hook: AppArmor,
> > SELinux, and the normal capability code. Looking at the function
> > implementations we may observe that the first parameter "target" is not
> > changing.
> >
> > Mark the first argument "target" of LSM hook security_capget(...) as
> > "const" since it will not be changing in the LSM hook.
>
>
> The commit message may be simplified, from this:
> [PATCH 1/2] lsm: change 'target' parameter to 'const' in security_capget LSM hook
>
> to something like this:
> [PATCH 1/2] lsm: Constify the target parameter in security_capget()
>
> "Constify" is the commonly used language in git logs.
> See git log --pretty=oneline --abbrev-commit | grep Constify
I'm not overly worried about the subject line; yes, Allison's
suggestion is an improvement, but the original is okay. However, I
would like to see patches 1/2 and 2/2 squashed together into a single
patch. If patch 1/2 hadn't already touched the function declaration
line that was too long there may have been an argument for keeping the
patchset as two patches, but since patch 1/2 does modify the function
declaration we might as well wrap that declaration line in that patch
too.
--
paul-moore.com