linux-kernel - Re: [PATCH 1/3] ptrace: ptrace_attach: check PF_KTHREAD + exit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20090510231112.5DABDFC35D@magilla.sf.frob.com>
Date:	Sun, 10 May 2009 16:11:12 -0700 (PDT)
From:	Roland McGrath <roland@...hat.com>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Chris Wright <chrisw@...s-sol.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] ptrace: ptrace_attach: check PF_KTHREAD +
	exit_state instead of ->mm

> > On second look, what does __ptrace_may_access() need task_lock() for anyway?
> 
> Just for get_dumpable(task->mm), I think.

Ah.  That's really only for using the mm, right?  i.e. it could be using
get_task_mm() instead.  set_dumpable() does not use task_lock() to
synchronize the actual changes that affect get_dumpable().

> OK, so this change is not purely cosmetic as I thought.

Right.

> We can fix this in many ways. We can extract the ->cred and ->mm checks
> from __ptrace_may_access() into another helper which is called before
> write_lock(tasklist), and then call security_ptrace_may_access under tasklist.
> Or we can do get_task_mm() in advance and call __ptrace_may_access() without
> task_lock().

get_task_mm() makes sense to me.  It seems like those checks being under
the tasklist_lock (i.e. the lock governing ptrace attach) might matter.

> Or, perhaps, we can just check ->ptrace before __ptrace_may_access()
> lockless (just to prevent the scenario above), and then check it again
> under tasklist? This looks like a simplest option.

Doesn't that still have a possible race with PTRACE_TRACEME?  If
ptrace_traceme() runs between the lockless check and the lock-taking, we
want PTRACE_ATTACH to fail without calling security_ptrace_may_access().
Maybe the cred mutex should be excluding that here anyway.

Thanks,
Roland
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/