[v2] security: Add LSM fixup hooks to set*gid syscalls.
diff mbox series

Message ID CAJ-EccNNCNrA7to85WrYFjRUP7Z1XC+sJ2JXkargiEvXd11AVg@mail.gmail.com
State New
Headers show
Series
  • [v2] security: Add LSM fixup hooks to set*gid syscalls.
Related show

Commit Message

Micah Morton July 31, 2018, 5:34 p.m. UTC
The set*uid system calls all call an LSM fixup hook called
security_task_fix_setuid, which allows for altering the behavior of those
calls by a security module. Comments explaining the LSM_SETID_* constants
in /include/linux/security.h imply that the constants are to be used for
both the set*uid and set*gid syscalls, but the set*gid syscalls do not
have the relevant hooks, meaning a security module can only alter syscalls
that change user identity attributes but not ones that change group
identity attributes. This patch adds the necessary LSM hook, called
security_task_fix_setgid, and calls the hook from the appropriate places
in the set*gid syscalls.Tested by putting a print statement in the hook and
seeing it triggered from the various set*gid syscalls.

Signed-off-by: Micah Morton <mortonm@chromium.org>
Acked-by: Kees Cook <keescook@chromium.org>
---
NOTE: the security_task_fix_setgid line in sys_setfsgid is over 80
characters, but I figured I'd just follow how it was done in sys_setfsuid
rather than trying to wrap the line, since the functions are nearly
identical.

  return call_int_hook(task_setpgid, 0, p, pgid);

--
4.18-rc2

--

Comments

Casey Schaufler July 31, 2018, 6:02 p.m. UTC | #1
On 7/31/2018 10:34 AM, Micah Morton wrote:
> The set*uid system calls all call an LSM fixup hook called
> security_task_fix_setuid, which allows for altering the behavior of those
> calls by a security module. Comments explaining the LSM_SETID_* constants
> in /include/linux/security.h imply that the constants are to be used for
> both the set*uid and set*gid syscalls, but the set*gid syscalls do not
> have the relevant hooks, meaning a security module can only alter syscalls
> that change user identity attributes but not ones that change group
> identity attributes. This patch adds the necessary LSM hook, called
> security_task_fix_setgid, and calls the hook from the appropriate places
> in the set*gid syscalls.Tested by putting a print statement in the hook and
> seeing it triggered from the various set*gid syscalls.
>
> Signed-off-by: Micah Morton <mortonm@chromium.org <mailto:mortonm@chromium.org>>
> Acked-by: Kees Cook <keescook@chromium.org <mailto:keescook@chromium.org>>

What security module is going to provide a hook for this?

> ---
> NOTE: the security_task_fix_setgid line in sys_setfsgid is over 80
> characters, but I figured I'd just follow how it was done in sys_setfsuid
> rather than trying to wrap the line, since the functions are nearly
> identical.
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 8f1131c8dd54..a2166c812a97 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -599,6 +599,15 @@
>   *@old is the set of credentials that are being replaces
>   *@flags contains one of the LSM_SETID_* values.
>   *Return 0 on success.
> + * @task_fix_setgid:
> + *     Update the module's state after setting one or more of the group
> + *     identity attributes of the current process.  The @flags parameter
> + *     indicates which of the set*gid system calls invoked this hook.
> + *     @new is the set of credentials that will be installed.  Modifications
> + *     should be made to this rather than to @current->cred.
> + *     @old is the set of credentials that are being replaced
> + *     @flags contains one of the LSM_SETID_* values.
> + *     Return 0 on success.
>   * @task_setpgid:
>   *Check permission before setting the process group identifier of the
>   *process @p to @pgid.
> @@ -1587,6 +1596,8 @@ union security_list_options {
>       enum kernel_read_file_id id);
>  int (*task_fix_setuid)(struct cred *new, const struct cred *old,
>  int flags);
> +int (*task_fix_setgid)(struct cred *new, const struct cred *old,
> +int flags);
>  int (*task_setpgid)(struct task_struct *p, pid_t pgid);
>  int (*task_getpgid)(struct task_struct *p);
>  int (*task_getsid)(struct task_struct *p);
> @@ -1876,6 +1887,7 @@ struct security_hook_heads {
>  struct hlist_head kernel_post_read_file;
>  struct hlist_head kernel_module_request;
>  struct hlist_head task_fix_setuid;
> +struct hlist_head task_fix_setgid;
>  struct hlist_head task_setpgid;
>  struct hlist_head task_getpgid;
>  struct hlist_head task_getsid;
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 63030c85ee19..a82d97cf13ab 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -325,6 +325,8 @@ int security_kernel_post_read_file(struct file *file, char *buf, loff_t size,
>     enum kernel_read_file_id id);
>  int security_task_fix_setuid(struct cred *new, const struct cred *old,
>       int flags);
> +int security_task_fix_setgid(struct cred *new, const struct cred *old,
> +     int flags);
>  int security_task_setpgid(struct task_struct *p, pid_t pgid);
>  int security_task_getpgid(struct task_struct *p);
>  int security_task_getsid(struct task_struct *p);
> @@ -929,6 +931,13 @@ static inline int security_task_fix_setuid(struct cred *new,
>  return cap_task_fix_setuid(new, old, flags);
>  }
>  
> +static inline int security_task_fix_setgid(struct cred *new,
> +   const struct cred *old,
> +   int flags)
> +{
> +return 0;
> +}
> +
>  static inline int security_task_setpgid(struct task_struct *p, pid_t pgid)
>  {
>  return 0;
> diff --git a/kernel/sys.c b/kernel/sys.c
> index 38509dc1f77b..f6ef922c6815 100644
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -392,6 +392,10 @@ long __sys_setregid(gid_t rgid, gid_t egid)
>  new->sgid = new->egid;
>  new->fsgid = new->egid;
>  
> +retval = security_task_fix_setgid(new, old, LSM_SETID_RE);
> +if (retval < 0)
> +goto error;
> +
>  return commit_creds(new);
>  
>  error:
> @@ -434,6 +438,10 @@ long __sys_setgid(gid_t gid)
>  else
>  goto error;
>  
> +retval = security_task_fix_setgid(new, old, LSM_SETID_ID);
> +if (retval < 0)
> +goto error;
> +
>  return commit_creds(new);
>  
>  error:
> @@ -755,6 +763,10 @@ long __sys_setresgid(gid_t rgid, gid_t egid, gid_t sgid)
>  new->sgid = ksgid;
>  new->fsgid = new->egid;
>  
> +retval = security_task_fix_setgid(new, old, LSM_SETID_RES);
> +if (retval < 0)
> +goto error;
> +
>  return commit_creds(new);
>  
>  error:
> @@ -861,7 +873,8 @@ long __sys_setfsgid(gid_t gid)
>      ns_capable(old->user_ns, CAP_SETGID)) {
>  if (!gid_eq(kgid, old->fsgid)) {
>  new->fsgid = kgid;
> -goto change_okay;
> +if (security_task_fix_setgid(new, old, LSM_SETID_FS) == 0)
> +goto change_okay;
>  }
>  }
>  
> diff --git a/security/security.c b/security/security.c
> index 68f46d849abe..587786fc0aaa 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1062,6 +1062,12 @@ int security_task_fix_setuid(struct cred *new, const struct cred *old,
>  return call_int_hook(task_fix_setuid, 0, new, old, flags);
>  }
>  
> +int security_task_fix_setgid(struct cred *new, const struct cred *old,
> +     int flags)
> +{
> +return call_int_hook(task_fix_setgid, 0, new, old, flags);
> +}
> +
>  int security_task_setpgid(struct task_struct *p, pid_t pgid)
>  {
>  return call_int_hook(task_setpgid, 0, p, pgid);
>
> --
> 4.18-rc2
>
> --
>
<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    On 7/31/2018 10:34 AM, Micah Morton wrote:<br>
    <blockquote type="cite"
cite="mid:CAJ-EccNNCNrA7to85WrYFjRUP7Z1XC+sJ2JXkargiEvXd11AVg@mail.gmail.com">
      <meta http-equiv="content-type" content="text/html; charset=utf-8">
      <div dir="ltr">
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">The
          set*uid system calls all call an LSM fixup hook called</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">security_task_fix_setuid,
          which allows for altering the behavior of those</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">calls
          by a security module. Comments explaining the LSM_SETID_*
          constants</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">in
          /include/linux/security.h imply that the constants are to be
          used for</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">both
          the set*uid and set*gid syscalls, but the set*gid syscalls do
          not</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">have
          the relevant hooks, meaning a security module can only alter
          syscalls</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">that
          change user identity attributes but not ones that change group</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">identity
          attributes. This patch adds the necessary LSM hook, called</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">security_task_fix_setgid,
          and calls the hook from the appropriate places</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">in
          the set*gid syscalls.Tested by putting a print statement in
          the hook and</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">seeing
          it triggered from the various set*gid syscalls.</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"><br>
        </div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">Signed-off-by:
          Micah Morton &lt;<a href="mailto:mortonm@chromium.org"
            target="_blank" style="color:rgb(17,85,204)"
            moz-do-not-send="true">mortonm@chromium.org</a>&gt;</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"><span
style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">Acked-by:
            Kees Cook &lt;</span><a href="mailto:keescook@chromium.org"
            target="_blank"
            style="color:rgb(17,85,204);background-color:rgb(255,255,255)"
            moz-do-not-send="true">keescook@chromium.org</a><span
style="background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">&gt;</span><br>
        </div>
      </div>
    </blockquote>
    <pre>
What security module is going to provide a hook for this?

</pre>
    <blockquote type="cite"
cite="mid:CAJ-EccNNCNrA7to85WrYFjRUP7Z1XC+sJ2JXkargiEvXd11AVg@mail.gmail.com">
      <div dir="ltr">
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">---</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">NOTE:
          the security_task_fix_setgid line in sys_setfsgid is over 80</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">characters,
          but I figured I'd just follow how it was done in sys_setfsuid</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">rather
          than trying to wrap the line, since the functions are nearly</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">identical.</div>
        <br
          class="gmail-m_4493492642882813245gmail-Apple-interchange-newline"
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">diff
          --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">index
          8f1131c8dd54..a2166c812a97 100644</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">---
          a/include/linux/lsm_hooks.h</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+++
          b/include/linux/lsm_hooks.h</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">@@
          -599,6 +599,15 @@</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> 
          *<span style="white-space:pre-wrap">	</span>@old is the set of
          credentials that are being replaces</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> 
          *<span style="white-space:pre-wrap">	</span>@flags contains
          one of the LSM_SETID_* values.</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> 
          *<span style="white-space:pre-wrap">	</span>Return 0 on
          success.</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+
          * @task_fix_setgid:</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+
          *     Update the module's state after setting one or more of
          the group</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+
          *     identity attributes of the current process.  The @flags
          parameter</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+
          *     indicates which of the set*gid system calls invoked this
          hook.</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+
          *     @new is the set of credentials that will be installed. 
          Modifications</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+
          *     should be made to this rather than to @current-&gt;cred.</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+
          *     @old is the set of credentials that are being replaced</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+
          *     @flags contains one of the LSM_SETID_* values.</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+
          *     Return 0 on success.</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> 
          * @task_setpgid:</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> 
          *<span style="white-space:pre-wrap">	</span>Check permission
          before setting the process group identifier of the</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> 
          *<span style="white-space:pre-wrap">	</span>process @p to
          @pgid.</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">@@
          -1587,6 +1596,8 @@ union security_list_options {</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> <span style="white-space:pre-wrap">				</span> 
             enum kernel_read_file_id id);</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> <span style="white-space:pre-wrap">	</span>int
          (*task_fix_setuid)(struct cred *new, const struct cred *old,</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> <span style="white-space:pre-wrap">				</span>int
          flags);</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+<span style="white-space:pre-wrap">	</span>int
          (*task_fix_setgid)(struct cred *new, const struct cred *old,</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+<span style="white-space:pre-wrap">				</span>int
          flags);</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> <span style="white-space:pre-wrap">	</span>int
          (*task_setpgid)(struct task_struct *p, pid_t pgid);</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> <span style="white-space:pre-wrap">	</span>int
          (*task_getpgid)(struct task_struct *p);</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> <span style="white-space:pre-wrap">	</span>int
          (*task_getsid)(struct task_struct *p);</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">@@
          -1876,6 +1887,7 @@ struct security_hook_heads {</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> <span style="white-space:pre-wrap">	</span>struct
          hlist_head kernel_post_read_file;</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> <span style="white-space:pre-wrap">	</span>struct
          hlist_head kernel_module_request;</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> <span style="white-space:pre-wrap">	</span>struct
          hlist_head task_fix_setuid;</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+<span style="white-space:pre-wrap">	</span>struct
          hlist_head task_fix_setgid;</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> <span style="white-space:pre-wrap">	</span>struct
          hlist_head task_setpgid;</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> <span style="white-space:pre-wrap">	</span>struct
          hlist_head task_getpgid;</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> <span style="white-space:pre-wrap">	</span>struct
          hlist_head task_getsid;</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">diff
          --git a/include/linux/security.h b/include/linux/security.h</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">index
          63030c85ee19..a82d97cf13ab 100644</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">---
          a/include/linux/security.h</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+++
          b/include/linux/security.h</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">@@
          -325,6 +325,8 @@ int security_kernel_post_read_file(struct
          file *file, char *buf, loff_t size,</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> <span style="white-space:pre-wrap">				</span> 
           enum kernel_read_file_id id);</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> int
          security_task_fix_setuid(struct cred *new, const struct cred
          *old,</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> <span style="white-space:pre-wrap">			</span> 
             int flags);</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+int
          security_task_fix_setgid(struct cred *new, const struct cred
          *old,</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+<span style="white-space:pre-wrap">			</span> 
             int flags);</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> int
          security_task_setpgid(struct task_struct *p, pid_t pgid);</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> int
          security_task_getpgid(struct task_struct *p);</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> int
          security_task_getsid(struct task_struct *p);</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">@@
          -929,6 +931,13 @@ static inline int
          security_task_fix_setuid(struct cred *new,</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> <span style="white-space:pre-wrap">	</span>return
          cap_task_fix_setuid(new, old, flags);</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> }</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> </div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+static
          inline int security_task_fix_setgid(struct cred *new,</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+<span style="white-space:pre-wrap">					</span> 
           const struct cred *old,</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+<span style="white-space:pre-wrap">					</span> 
           int flags)</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+{</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+<span style="white-space:pre-wrap">	</span>return
          0;</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+}</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> static
          inline int security_task_setpgid(struct task_struct *p, pid_t
          pgid)</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> {</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> <span style="white-space:pre-wrap">	</span>return
          0;</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">diff
          --git a/kernel/sys.c b/kernel/sys.c</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">index
          38509dc1f77b..f6ef922c6815 100644</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">---
          a/kernel/sys.c</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+++
          b/kernel/sys.c</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">@@
          -392,6 +392,10 @@ long __sys_setregid(gid_t rgid, gid_t egid)</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> <span style="white-space:pre-wrap">		</span>new-&gt;sgid
          = new-&gt;egid;</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> <span style="white-space:pre-wrap">	</span>new-&gt;fsgid
          = new-&gt;egid;</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> </div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+<span style="white-space:pre-wrap">	</span>retval
          = security_task_fix_setgid(new, old, LSM_SETID_RE);</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+<span style="white-space:pre-wrap">	</span>if
          (retval &lt; 0)</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+<span style="white-space:pre-wrap">		</span>goto
          error;</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> <span style="white-space:pre-wrap">	</span>return
          commit_creds(new);</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> </div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> error:</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">@@
          -434,6 +438,10 @@ long __sys_setgid(gid_t gid)</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> <span style="white-space:pre-wrap">	</span>else</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> <span style="white-space:pre-wrap">		</span>goto
          error;</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> </div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+<span style="white-space:pre-wrap">	</span>retval
          = security_task_fix_setgid(new, old, LSM_SETID_ID);</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+<span style="white-space:pre-wrap">	</span>if
          (retval &lt; 0)</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+<span style="white-space:pre-wrap">		</span>goto
          error;</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> <span style="white-space:pre-wrap">	</span>return
          commit_creds(new);</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> </div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> error:</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">@@
          -755,6 +763,10 @@ long __sys_setresgid(gid_t rgid, gid_t egid,
          gid_t sgid)</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> <span style="white-space:pre-wrap">		</span>new-&gt;sgid
          = ksgid;</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> <span style="white-space:pre-wrap">	</span>new-&gt;fsgid
          = new-&gt;egid;</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> </div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+<span style="white-space:pre-wrap">	</span>retval
          = security_task_fix_setgid(new, old, LSM_SETID_RES);</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+<span style="white-space:pre-wrap">	</span>if
          (retval &lt; 0)</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+<span style="white-space:pre-wrap">		</span>goto
          error;</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> <span style="white-space:pre-wrap">	</span>return
          commit_creds(new);</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> </div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> error:</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">@@
          -861,7 +873,8 @@ long __sys_setfsgid(gid_t gid)</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> <span style="white-space:pre-wrap">	</span> 
            ns_capable(old-&gt;user_ns, CAP_SETGID)) {</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> <span style="white-space:pre-wrap">		</span>if
          (!gid_eq(kgid, old-&gt;fsgid)) {</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> <span style="white-space:pre-wrap">			</span>new-&gt;fsgid
          = kgid;</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">-<span style="white-space:pre-wrap">			</span>goto
          change_okay;</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+<span style="white-space:pre-wrap">			</span>if
          (security_task_fix_setgid(new, old, LSM_SETID_FS) == 0)</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+<span style="white-space:pre-wrap">				</span>goto
          change_okay;</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> <span style="white-space:pre-wrap">		</span>}</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> <span style="white-space:pre-wrap">	</span>}</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> </div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">diff
          --git a/security/security.c b/security/security.c</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">index
          68f46d849abe..587786fc0aaa 100644</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">---
          a/security/security.c</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+++
          b/security/security.c</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">@@
          -1062,6 +1062,12 @@ int security_task_fix_setuid(struct cred
          *new, const struct cred *old,</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> <span style="white-space:pre-wrap">	</span>return
          call_int_hook(task_fix_setuid, 0, new, old, flags);</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> }</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> </div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+int
          security_task_fix_setgid(struct cred *new, const struct cred
          *old,</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+<span style="white-space:pre-wrap">			</span> 
             int flags)</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+{</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+<span style="white-space:pre-wrap">	</span>return
          call_int_hook(task_fix_setgid, 0, new, old, flags);</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+}</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">+</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> int
          security_task_setpgid(struct task_struct *p, pid_t pgid)</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> {</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"> <span style="white-space:pre-wrap">	</span>return
          call_int_hook(task_setpgid, 0, p, pgid);</div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial"><br>
        </div>
        <div
style="font-size:small;text-decoration-style:initial;text-decoration-color:initial">
          <div
style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial">--</div>
          <div
style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial">4.18-rc2<br>
          </div>
          <div
style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial"><br>
          </div>
          <div
style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial">--</div>
        </div>
        <br>
      </div>
    </blockquote>
    <br>
  </body>
</html>
James Morris July 31, 2018, 8:08 p.m. UTC | #2
On Tue, 31 Jul 2018, Micah Morton wrote:

> +static inline int security_task_fix_setgid(struct cred *new,
> +    const struct cred *old,
> +    int flags)
> +{
> + return 0;
> +}
> +

This looks whitespace-damaged.  Please send patches as plain text.
Micah Morton July 31, 2018, 9:47 p.m. UTC | #3
The ChromiumOS LSM used by ChromeOS will provide a hook for this, in
order to enforce ChromeOS-specific policies regarding which UIDs/GIDs a
process with CAP_SET{UID/GID} can transition to. The
security_task_fix_setuid LSM hook is very helpful in enabling such a feature
for ChromeOS that governs UID transitions, but unfortunately for us it looks
like the equivalent hook that would allow us to govern GID transitions from an
LSM never got added.

Resending with plain text mode enabled.

---

The set*uid system calls all call an LSM fixup hook called
security_task_fix_setuid, which allows for altering the behavior of those
calls by a security module. Comments explaining the LSM_SETID_* constants
in /include/linux/security.h imply that the constants are to be used for
both the set*uid and set*gid syscalls, but the set*gid syscalls do not
have the relevant hooks, meaning a security module can only alter syscalls
that change user identity attributes but not ones that change group
identity attributes. This patch adds the necessary LSM hook, called
security_task_fix_setgid, and calls the hook from the appropriate places
in the set*gid syscalls.Tested by putting a print statement in the hook and
seeing it triggered from the various set*gid syscalls.

Signed-off-by: Micah Morton <mortonm@chromium.org>
Acked-by: Kees Cook <keescook@chromium.org>
---
NOTE: the security_task_fix_setgid line in sys_setfsgid is over 80
characters, but I figured I'd just follow how it was done in sys_setfsuid
rather than trying to wrap the line, since the functions are nearly
identical.

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 8f1131c8dd54..a2166c812a97 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -599,6 +599,15 @@
  * @old is the set of credentials that are being replaces
  * @flags contains one of the LSM_SETID_* values.
  * Return 0 on success.
+ * @task_fix_setgid:
+ *     Update the module's state after setting one or more of the group
+ *     identity attributes of the current process.  The @flags parameter
+ *     indicates which of the set*gid system calls invoked this hook.
+ *     @new is the set of credentials that will be installed.  Modifications
+ *     should be made to this rather than to @current->cred.
+ *     @old is the set of credentials that are being replaced
+ *     @flags contains one of the LSM_SETID_* values.
+ *     Return 0 on success.
  * @task_setpgid:
  * Check permission before setting the process group identifier of the
  * process @p to @pgid.
@@ -1587,6 +1596,8 @@ union security_list_options {
       enum kernel_read_file_id id);
  int (*task_fix_setuid)(struct cred *new, const struct cred *old,
  int flags);
+ int (*task_fix_setgid)(struct cred *new, const struct cred *old,
+ int flags);
  int (*task_setpgid)(struct task_struct *p, pid_t pgid);
  int (*task_getpgid)(struct task_struct *p);
  int (*task_getsid)(struct task_struct *p);
@@ -1876,6 +1887,7 @@ struct security_hook_heads {
  struct hlist_head kernel_post_read_file;
  struct hlist_head kernel_module_request;
  struct hlist_head task_fix_setuid;
+ struct hlist_head task_fix_setgid;
  struct hlist_head task_setpgid;
  struct hlist_head task_getpgid;
  struct hlist_head task_getsid;
diff --git a/include/linux/security.h b/include/linux/security.h
index 63030c85ee19..a82d97cf13ab 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -325,6 +325,8 @@ int security_kernel_post_read_file(struct file
*file, char *buf, loff_t size,
     enum kernel_read_file_id id);
 int security_task_fix_setuid(struct cred *new, const struct cred *old,
       int flags);
+int security_task_fix_setgid(struct cred *new, const struct cred *old,
+      int flags);
 int security_task_setpgid(struct task_struct *p, pid_t pgid);
 int security_task_getpgid(struct task_struct *p);
 int security_task_getsid(struct task_struct *p);
@@ -929,6 +931,13 @@ static inline int security_task_fix_setuid(struct
cred *new,
  return cap_task_fix_setuid(new, old, flags);
 }

+static inline int security_task_fix_setgid(struct cred *new,
+    const struct cred *old,
+    int flags)
+{
+ return 0;
+}
+
 static inline int security_task_setpgid(struct task_struct *p, pid_t pgid)
 {
  return 0;
diff --git a/kernel/sys.c b/kernel/sys.c
index 38509dc1f77b..f6ef922c6815 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -392,6 +392,10 @@ long __sys_setregid(gid_t rgid, gid_t egid)
  new->sgid = new->egid;
  new->fsgid = new->egid;

+ retval = security_task_fix_setgid(new, old, LSM_SETID_RE);
+ if (retval < 0)
+ goto error;
+
  return commit_creds(new);

 error:
@@ -434,6 +438,10 @@ long __sys_setgid(gid_t gid)
  else
  goto error;

+ retval = security_task_fix_setgid(new, old, LSM_SETID_ID);
+ if (retval < 0)
+ goto error;
+
  return commit_creds(new);

 error:
@@ -755,6 +763,10 @@ long __sys_setresgid(gid_t rgid, gid_t egid, gid_t sgid)
  new->sgid = ksgid;
  new->fsgid = new->egid;

+ retval = security_task_fix_setgid(new, old, LSM_SETID_RES);
+ if (retval < 0)
+ goto error;
+
  return commit_creds(new);

 error:
@@ -861,7 +873,8 @@ long __sys_setfsgid(gid_t gid)
      ns_capable(old->user_ns, CAP_SETGID)) {
  if (!gid_eq(kgid, old->fsgid)) {
  new->fsgid = kgid;
- goto change_okay;
+ if (security_task_fix_setgid(new, old, LSM_SETID_FS) == 0)
+ goto change_okay;
  }
  }

diff --git a/security/security.c b/security/security.c
index 68f46d849abe..587786fc0aaa 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1062,6 +1062,12 @@ int security_task_fix_setuid(struct cred *new,
const struct cred *old,
  return call_int_hook(task_fix_setuid, 0, new, old, flags);
 }

+int security_task_fix_setgid(struct cred *new, const struct cred *old,
+      int flags)
+{
+ return call_int_hook(task_fix_setgid, 0, new, old, flags);
+}
+
 int security_task_setpgid(struct task_struct *p, pid_t pgid)
 {
  return call_int_hook(task_setpgid, 0, p, pgid);

--
4.18-rc2

--

On Tue, Jul 31, 2018 at 1:09 PM James Morris <jmorris@namei.org> wrote:
>
> On Tue, 31 Jul 2018, Micah Morton wrote:
>
> > +static inline int security_task_fix_setgid(struct cred *new,
> > +    const struct cred *old,
> > +    int flags)
> > +{
> > + return 0;
> > +}
> > +
>
> This looks whitespace-damaged.  Please send patches as plain text.
>
>
> --
> James Morris
> <jmorris@namei.org>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Randy Dunlap July 31, 2018, 10:06 p.m. UTC | #4
On 07/31/2018 02:47 PM, Micah Morton wrote:
> The ChromiumOS LSM used by ChromeOS will provide a hook for this, in
> order to enforce ChromeOS-specific policies regarding which UIDs/GIDs a
> process with CAP_SET{UID/GID} can transition to. The
> security_task_fix_setuid LSM hook is very helpful in enabling such a feature
> for ChromeOS that governs UID transitions, but unfortunately for us it looks
> like the equivalent hook that would allow us to govern GID transitions from an
> LSM never got added.
> 
> Resending with plain text mode enabled.
> 
> ---
> 
> The set*uid system calls all call an LSM fixup hook called
> security_task_fix_setuid, which allows for altering the behavior of those
> calls by a security module. Comments explaining the LSM_SETID_* constants
> in /include/linux/security.h imply that the constants are to be used for
> both the set*uid and set*gid syscalls, but the set*gid syscalls do not
> have the relevant hooks, meaning a security module can only alter syscalls
> that change user identity attributes but not ones that change group
> identity attributes. This patch adds the necessary LSM hook, called
> security_task_fix_setgid, and calls the hook from the appropriate places
> in the set*gid syscalls.Tested by putting a print statement in the hook and
> seeing it triggered from the various set*gid syscalls.
> 
> Signed-off-by: Micah Morton <mortonm@chromium.org>
> Acked-by: Kees Cook <keescook@chromium.org>
> ---
> NOTE: the security_task_fix_setgid line in sys_setfsgid is over 80
> characters, but I figured I'd just follow how it was done in sys_setfsuid
> rather than trying to wrap the line, since the functions are nearly
> identical.
> 
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 8f1131c8dd54..a2166c812a97 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -599,6 +599,15 @@
>   * @old is the set of credentials that are being replaces
>   * @flags contains one of the LSM_SETID_* values.
>   * Return 0 on success.
> + * @task_fix_setgid:
> + *     Update the module's state after setting one or more of the group
> + *     identity attributes of the current process.  The @flags parameter
> + *     indicates which of the set*gid system calls invoked this hook.
> + *     @new is the set of credentials that will be installed.  Modifications
> + *     should be made to this rather than to @current->cred.
> + *     @old is the set of credentials that are being replaced
> + *     @flags contains one of the LSM_SETID_* values.
> + *     Return 0 on success.
>   * @task_setpgid:
>   * Check permission before setting the process group identifier of the
>   * process @p to @pgid.
> @@ -1587,6 +1596,8 @@ union security_list_options {
>        enum kernel_read_file_id id);
>   int (*task_fix_setuid)(struct cred *new, const struct cred *old,
>   int flags);
> + int (*task_fix_setgid)(struct cred *new, const struct cred *old,
> + int flags);
>   int (*task_setpgid)(struct task_struct *p, pid_t pgid);
>   int (*task_getpgid)(struct task_struct *p);
>   int (*task_getsid)(struct task_struct *p);
> @@ -1876,6 +1887,7 @@ struct security_hook_heads {
>   struct hlist_head kernel_post_read_file;
>   struct hlist_head kernel_module_request;
>   struct hlist_head task_fix_setuid;
> + struct hlist_head task_fix_setgid;
>   struct hlist_head task_setpgid;
>   struct hlist_head task_getpgid;
>   struct hlist_head task_getsid;
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 63030c85ee19..a82d97cf13ab 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -325,6 +325,8 @@ int security_kernel_post_read_file(struct file
> *file, char *buf, loff_t size,
>      enum kernel_read_file_id id);
>  int security_task_fix_setuid(struct cred *new, const struct cred *old,
>        int flags);
> +int security_task_fix_setgid(struct cred *new, const struct cred *old,
> +      int flags);
>  int security_task_setpgid(struct task_struct *p, pid_t pgid);
>  int security_task_getpgid(struct task_struct *p);
>  int security_task_getsid(struct task_struct *p);
> @@ -929,6 +931,13 @@ static inline int security_task_fix_setuid(struct
> cred *new,
>   return cap_task_fix_setuid(new, old, flags);
>  }
> 
> +static inline int security_task_fix_setgid(struct cred *new,
> +    const struct cred *old,
> +    int flags)
> +{
> + return 0;
> +}
> +
>  static inline int security_task_setpgid(struct task_struct *p, pid_t pgid)
>  {
>   return 0;
> diff --git a/kernel/sys.c b/kernel/sys.c
> index 38509dc1f77b..f6ef922c6815 100644
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -392,6 +392,10 @@ long __sys_setregid(gid_t rgid, gid_t egid)
>   new->sgid = new->egid;
>   new->fsgid = new->egid;
> 
> + retval = security_task_fix_setgid(new, old, LSM_SETID_RE);
> + if (retval < 0)
> + goto error;
> +
>   return commit_creds(new);
> 
>  error:
> @@ -434,6 +438,10 @@ long __sys_setgid(gid_t gid)
>   else
>   goto error;
> 
> + retval = security_task_fix_setgid(new, old, LSM_SETID_ID);
> + if (retval < 0)
> + goto error;
> +
>   return commit_creds(new);
> 
>  error:
> @@ -755,6 +763,10 @@ long __sys_setresgid(gid_t rgid, gid_t egid, gid_t sgid)
>   new->sgid = ksgid;
>   new->fsgid = new->egid;
> 
> + retval = security_task_fix_setgid(new, old, LSM_SETID_RES);
> + if (retval < 0)
> + goto error;
> +
>   return commit_creds(new);
> 
>  error:
> @@ -861,7 +873,8 @@ long __sys_setfsgid(gid_t gid)
>       ns_capable(old->user_ns, CAP_SETGID)) {
>   if (!gid_eq(kgid, old->fsgid)) {
>   new->fsgid = kgid;
> - goto change_okay;
> + if (security_task_fix_setgid(new, old, LSM_SETID_FS) == 0)
> + goto change_okay;
>   }
>   }
> 
> diff --git a/security/security.c b/security/security.c
> index 68f46d849abe..587786fc0aaa 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1062,6 +1062,12 @@ int security_task_fix_setuid(struct cred *new,
> const struct cred *old,
>   return call_int_hook(task_fix_setuid, 0, new, old, flags);
>  }
> 
> +int security_task_fix_setgid(struct cred *new, const struct cred *old,
> +      int flags)
> +{
> + return call_int_hook(task_fix_setgid, 0, new, old, flags);
> +}
> +
>  int security_task_setpgid(struct task_struct *p, pid_t pgid)
>  {
>   return call_int_hook(task_setpgid, 0, p, pgid);
> 
> --
> 4.18-rc2
> 
> --
> 
> On Tue, Jul 31, 2018 at 1:09 PM James Morris <jmorris@namei.org> wrote:
>>
>> On Tue, 31 Jul 2018, Micah Morton wrote:
>>
>>> +static inline int security_task_fix_setgid(struct cred *new,
>>> +    const struct cred *old,
>>> +    int flags)
>>> +{
>>> + return 0;
>>> +}
>>> +
>>
>> This looks whitespace-damaged.  Please send patches as plain text.

Still is ...
James Morris Aug. 1, 2018, 7:34 p.m. UTC | #5
On Tue, 31 Jul 2018, Micah Morton wrote:

> The ChromiumOS LSM used by ChromeOS will provide a hook for this, in
> order to enforce ChromeOS-specific policies regarding which UIDs/GIDs a
> process with CAP_SET{UID/GID} can transition to

Will you be submitting this LSM to mainline?  It's a policy generally of 
the kernel that we only add features to support in-tree code.

Patch
diff mbox series

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 8f1131c8dd54..a2166c812a97 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -599,6 +599,15 @@ 
  * @old is the set of credentials that are being replaces
  * @flags contains one of the LSM_SETID_* values.
  * Return 0 on success.
+ * @task_fix_setgid:
+ *     Update the module's state after setting one or more of the group
+ *     identity attributes of the current process.  The @flags parameter
+ *     indicates which of the set*gid system calls invoked this hook.
+ *     @new is the set of credentials that will be installed.
Modifications
+ *     should be made to this rather than to @current->cred.
+ *     @old is the set of credentials that are being replaced
+ *     @flags contains one of the LSM_SETID_* values.
+ *     Return 0 on success.
  * @task_setpgid:
  * Check permission before setting the process group identifier of the
  * process @p to @pgid.
@@ -1587,6 +1596,8 @@  union security_list_options {
       enum kernel_read_file_id id);
  int (*task_fix_setuid)(struct cred *new, const struct cred *old,
  int flags);
+ int (*task_fix_setgid)(struct cred *new, const struct cred *old,
+ int flags);
  int (*task_setpgid)(struct task_struct *p, pid_t pgid);
  int (*task_getpgid)(struct task_struct *p);
  int (*task_getsid)(struct task_struct *p);
@@ -1876,6 +1887,7 @@  struct security_hook_heads {
  struct hlist_head kernel_post_read_file;
  struct hlist_head kernel_module_request;
  struct hlist_head task_fix_setuid;
+ struct hlist_head task_fix_setgid;
  struct hlist_head task_setpgid;
  struct hlist_head task_getpgid;
  struct hlist_head task_getsid;
diff --git a/include/linux/security.h b/include/linux/security.h
index 63030c85ee19..a82d97cf13ab 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -325,6 +325,8 @@  int security_kernel_post_read_file(struct file *file,
char *buf, loff_t size,
     enum kernel_read_file_id id);
 int security_task_fix_setuid(struct cred *new, const struct cred *old,
       int flags);
+int security_task_fix_setgid(struct cred *new, const struct cred *old,
+      int flags);
 int security_task_setpgid(struct task_struct *p, pid_t pgid);
 int security_task_getpgid(struct task_struct *p);
 int security_task_getsid(struct task_struct *p);
@@ -929,6 +931,13 @@  static inline int security_task_fix_setuid(struct cred
*new,
  return cap_task_fix_setuid(new, old, flags);
 }

+static inline int security_task_fix_setgid(struct cred *new,
+    const struct cred *old,
+    int flags)
+{
+ return 0;
+}
+
 static inline int security_task_setpgid(struct task_struct *p, pid_t pgid)
 {
  return 0;
diff --git a/kernel/sys.c b/kernel/sys.c
index 38509dc1f77b..f6ef922c6815 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -392,6 +392,10 @@  long __sys_setregid(gid_t rgid, gid_t egid)
  new->sgid = new->egid;
  new->fsgid = new->egid;

+ retval = security_task_fix_setgid(new, old, LSM_SETID_RE);
+ if (retval < 0)
+ goto error;
+
  return commit_creds(new);

 error:
@@ -434,6 +438,10 @@  long __sys_setgid(gid_t gid)
  else
  goto error;

+ retval = security_task_fix_setgid(new, old, LSM_SETID_ID);
+ if (retval < 0)
+ goto error;
+
  return commit_creds(new);

 error:
@@ -755,6 +763,10 @@  long __sys_setresgid(gid_t rgid, gid_t egid, gid_t
sgid)
  new->sgid = ksgid;
  new->fsgid = new->egid;

+ retval = security_task_fix_setgid(new, old, LSM_SETID_RES);
+ if (retval < 0)
+ goto error;
+
  return commit_creds(new);

 error:
@@ -861,7 +873,8 @@  long __sys_setfsgid(gid_t gid)
      ns_capable(old->user_ns, CAP_SETGID)) {
  if (!gid_eq(kgid, old->fsgid)) {
  new->fsgid = kgid;
- goto change_okay;
+ if (security_task_fix_setgid(new, old, LSM_SETID_FS) == 0)
+ goto change_okay;
  }
  }

diff --git a/security/security.c b/security/security.c
index 68f46d849abe..587786fc0aaa 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1062,6 +1062,12 @@  int security_task_fix_setuid(struct cred *new, const
struct cred *old,
  return call_int_hook(task_fix_setuid, 0, new, old, flags);
 }

+int security_task_fix_setgid(struct cred *new, const struct cred *old,
+      int flags)
+{
+ return call_int_hook(task_fix_setgid, 0, new, old, flags);
+}
+
 int security_task_setpgid(struct task_struct *p, pid_t pgid)
 {