Thread (24 messages) 24 messages, 5 authors, 2020-02-12

Re: [PATCH v8 08/11] proc: instantiate only pids that we can ptrace on 'hidepid=4' mount option

From: Alexey Gladkov <hidden>
Date: 2020-02-12 14:34:11
Also in: linux-fsdevel, linux-security-module, lkml

On Mon, Feb 10, 2020 at 04:29:31PM +0000, Jordan Glover wrote:
On Monday, February 10, 2020 3:05 PM, Alexey Gladkov [off-list ref] wrote:
quoted
If "hidepid=4" mount option is set then do not instantiate pids that
we can not ptrace. "hidepid=4" means that procfs should only contain
pids that the caller can ptrace.

Cc: Kees Cook keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org
Cc: Andy Lutomirski luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Signed-off-by: Djalal Harouni tixxdz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Signed-off-by: Alexey Gladkov gladkov.alexey-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

fs/proc/base.c | 15 +++++++++++++++
fs/proc/root.c | 14 +++++++++++---
include/linux/proc_fs.h | 1 +
3 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 24b7c620ded3..49937d54e745 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -699,6 +699,14 @@ static bool has_pid_permissions(struct proc_fs_info *fs_info,
struct task_struct *task,
int hide_pid_min)
{

-   /*
-   -   If 'hidpid' mount option is set force a ptrace check,
-   -   we indicate that we are using a filesystem syscall
-   -   by passing PTRACE_MODE_READ_FSCREDS
-   */
-   if (proc_fs_hide_pid(fs_info) == HIDEPID_NOT_PTRACABLE)
-         return ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS);


-   if (proc_fs_hide_pid(fs_info) < hide_pid_min)
    return true;
    if (in_group_p(proc_fs_pid_gid(fs_info)))
    @@ -3271,7 +3279,14 @@ struct dentry *proc_pid_lookup(struct dentry *dentry, unsigned int flags)
    if (!task)
    goto out;

-   /* Limit procfs to only ptracable tasks */
-   if (proc_fs_hide_pid(fs_info) == HIDEPID_NOT_PTRACABLE) {
-         if (!has_pid_permissions(fs_info, task, HIDEPID_NO_ACCESS))


-         	goto out_put_task;


-   }
-   result = proc_pid_instantiate(dentry, task, NULL);
    +out_put_task:
    put_task_struct(task);
    out:
    return result;
    diff --git a/fs/proc/root.c b/fs/proc/root.c
    index e2bb015da1a8..5e27bb31f125 100644
    --- a/fs/proc/root.c
    +++ b/fs/proc/root.c
    @@ -52,6 +52,15 @@ static const struct fs_parameter_description proc_fs_parameters = {
    .specs = proc_param_specs,
    };

    +static inline int
    +valid_hidepid(unsigned int value)
    +{

-   return (value == HIDEPID_OFF ||
-         value == HIDEPID_NO_ACCESS ||


-         value == HIDEPID_INVISIBLE ||


-         value == HIDEPID_NOT_PTRACABLE);



+}
+
static int proc_parse_param(struct fs_context *fc, struct fs_parameter *param)
{
struct proc_fs_context *ctx = fc->fs_private;
@@ -68,10 +77,9 @@ static int proc_parse_param(struct fs_context *fc, struct fs_parameter *param)
break;

case Opt_hidepid:

-         if (!valid_hidepid(result.uint_32))


-         	return invalf(fc, "proc: unknown value of hidepid.\\n");
          ctx->hidepid = result.uint_32;



-         if (ctx->hidepid < HIDEPID_OFF ||


-             ctx->hidepid > HIDEPID_INVISIBLE)


-         	return invalf(fc, "proc: hidepid value must be between 0 and 2.\\n");
          break;



default:
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index f307940f8311..6822548405a7 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -17,6 +17,7 @@ enum {
HIDEPID_OFF = 0,
HIDEPID_NO_ACCESS = 1,
HIDEPID_INVISIBLE = 2,

-   HIDEPID_NOT_PTRACABLE = 4, /* Limit pids to only ptracable pids */
Is there a reason new option is "4" instead of "3"? The order 1..2..4 may be
confusing for people.
This is just mask. For now hidepid values are mutually exclusive, but
since it moved to uapi, I thought it would be good if there was an
opportunity to combine values.

-- 
Rgrds, legion
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help