On 1/15/24 20:37, Matthew Wilcox wrote:
> 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.
>
Ah, okay, never mind.
A couple new email addresses were found this time when I used
./scripts/get_maintainer.pl
>> +++ 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:
Apparently yes, but I think I only added those lines to silence
some automatic checking bots.
>
> /**
> * 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.
> */
>
Thanks a lot, that looks much better. I will use your suggestion as is,
when I re-send the patch next time.
>> @@ -508,6 +531,14 @@ static int ptrace_traceme(void)
>> {
>> int ret = -EPERM;
>>
>> + if (mutex_lock_interruptible(¤t->signal->cred_guard_mutex))
>> + return -ERESTARTNOINTR;
>
> Do you really want this to be interruptible by a timer signal or a
> window resize event?
>
I think that is kind of okay, as most of the existing users lock the mutex
also interruptible, so I just wanted to follow those examples.
Thanks
Bernd.
