lkml.org 
[lkml]   [2024]   [Jan]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH v14] exec: Fix dead-lock in de_thread with ptrace_attach
On Mon, Jan 15, 2024 at 08:22:19PM +0100, Bernd Edlinger wrote:
> This introduces signal->exec_bprm, which is used to
> fix the case when at least one of the sibling threads
> is traced, and therefore the trace process may dead-lock
> in ptrace_attach, but de_thread will need to wait for the
> tracer to continue execution.

Not entirely sure why I've been added to the cc; this doesn't seem
like it's even remotely within my realm of expertise.

> +++ b/include/linux/cred.h
> @@ -153,6 +153,7 @@ extern const struct cred *get_task_cred(struct task_struct *);
> extern struct cred *cred_alloc_blank(void);
> extern struct cred *prepare_creds(void);
> extern struct cred *prepare_exec_creds(void);
> +extern bool is_dumpability_changed(const struct cred *, const struct cred *);

Using 'extern' for function declarations is deprecated. More
importantly, you have two arguments of the same type, and how do I know
which one is which if you don't name them?

> +++ b/kernel/cred.c
> @@ -375,6 +375,28 @@ static bool cred_cap_issubset(const struct cred *set, const struct cred *subset)
> return false;
> }
>
> +/**
> + * is_dumpability_changed - Will changing creds from old to new
> + * affect the dumpability in commit_creds?
> + *
> + * Return: false - dumpability will not be changed in commit_creds.
> + * true - dumpability will be changed to non-dumpable.
> + *
> + * @old: The old credentials
> + * @new: The new credentials
> + */

Does kernel-doc really parse this correctly? Normal style would be:

/**
* is_dumpability_changed - Will changing creds affect dumpability?
* @old: The old credentials.
* @new: The new credentials.
*
* If the @new credentials have no elevated privileges compared to the
* @old credentials, the task may remain dumpable. Otherwise we have
* to mark the task as undumpable to avoid information leaks from higher
* to lower privilege domains.
*
* Return: True if the task will become undumpable.
*/

> @@ -508,6 +531,14 @@ static int ptrace_traceme(void)
> {
> int ret = -EPERM;
>
> + if (mutex_lock_interruptible(&current->signal->cred_guard_mutex))
> + return -ERESTARTNOINTR;

Do you really want this to be interruptible by a timer signal or a
window resize event?


\
 
 \ /
  Last update: 2024-01-15 20:44    [W:0.088 / U:0.132 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site