Re: [PATCH v2] lsm: constify the 'target' parameter in security_capget()

Khadija Kamran
2023-08-08T20:54:58Z

On Mon, Aug 07, 2023 at 07:09:33PM -0400, Paul Moore wrote:
> On Mon, Aug 7, 2023 at 2:59 AM Khadija Kamran <kamrankhadijadj@gmail.com> wrote:
> >
> >
> >
> > cap_capget() LSM hook declaration exceeds the 80 characters per line
> > limit. Split the function declaration to multple lines to decrease the
>
> "multiple" :)
>
> Don't worry, I'll fix that in the merge.
>

Hey Paul,
Thank you. :)

> There is one more nitpick below, but I think this looks good. I'll
> give this a few days to see if John can ACK the AppArmor bits, but if
> we don't hear from him by mid-week I'll plan to merge this.
>
> Thanks!
>
> > line length.
> >
> > Signed-off-by: Khadija Kamran <kamrankhadijadj@gmail.com>
> > ---
> > Changes in v2:
> > - Squash the patches 1/2 and 2/2 into a single patch
> > - Simplify the commit message
> >
> > 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(-)
> >
> > 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..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(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);
>
> Don't resubmit the patch just for this, I'll fix it during the merge,
> but generally nice to make sure the wrapped lines are aligned with the
> previous line; look at the security_cap_get() declaration (below) in
> this patch to see an example.

Okay noted.

Regards,
Khadija

>
> > extern int cap_capset(struct cred *new, const struct cred *old,
> > const kernel_cap_t *effective,
> > const kernel_cap_t *inheritable,
> > @@ -271,7 +272,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 +554,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
>
> --
> paul-moore.com