[Devel] [PATCH vz9] ucounts: Fix ucount incr/decr when uid is changed without changing the underlying user_struct

[nb]

On 9.03.23 г. 12:37 ч., Nikolay Borisov wrote:
> When preforming setuid the general expectation is that the underlying
> user_struct will also be changed. However, in our kernels this doesn't
> hold true to the presence of ff716deacf0c ("userns: associate user_struct
> with the user_namespace").
> 
> Running userns08 testcase from ltp causes a warning to be triggered
> when decrementing NPROC count on process reap. The reason for the
> warning is when the uid is changed the relevant code in commit_cred()
> is supposed to adjust the ucounts for the old (pre-setuid) and new
> (after setuid) state. However, the ceck in the upstream kernel only
> triggers if userns or user_struct have changed. Fix the issue by
> augmenting the check to ensure NPROC is adjusted when just the uid is
> changed.
> 
> https://jira.sw.ru/browse/PSBM-145641
> Signed-off-by: Nikolay Borisov <nikolay.borisov at virtuozzo.com>
> ---
>   kernel/cred.c | 16 ++++++++++++++--
>   1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/cred.c b/kernel/cred.c
> index 96466cc1c527..7dedd97a27c2 100644
> --- a/kernel/cred.c
> +++ b/kernel/cred.c
> @@ -395,6 +395,9 @@ int copy_creds(struct task_struct *p, unsigned long clone_flags)
>   #endif
> 
>   	p->cred = p->real_cred = get_cred(new);
> +	trace_printk("incr rlim for ns: %u val: %ld uid: %u euid: %u ucounts: %p\n", task_ucounts(p)->ns->ns.inum,
> +		     atomic_long_read(&task_ucounts(p)->ucount[UCOUNT_RLIMIT_NPROC]),
> +		     task_uid(p).val, task_euid(p).val, task_ucounts(p));
>   	inc_rlimit_ucounts(task_ucounts(p), UCOUNT_RLIMIT_NPROC, 1);
>   	alter_cred_subscribers(new, 2);
>   	validate_creds(new);
> @@ -495,12 +498,21 @@ int commit_creds(struct cred *new)
>   	 * in set_user().
>   	 */
>   	alter_cred_subscribers(new, 2);
> -	if (new->user != old->user || new->user_ns != old->user_ns)
> +	trace_printk("User adjustment\n");
> +	if (new->user != old->user || new->user_ns != old->user_ns || !uid_eq(new->uid, old->uid)) {
> +		trace_printk("incr rlim for ns: %u val: %ld nuid: %u ouid: %u new ucount: %p old ucount: %p\n", new->ucounts->ns->ns.inum,
> +			     atomic_long_read(&new->ucounts->ucount[UCOUNT_RLIMIT_NPROC]), __kuid_val(new->uid), __kuid_val(old->uid),
> +			     new->ucounts, old->ucounts);

Grrr, those trace_printk's calls were used during debugging and weren't 
supposed to be included in the patch. Konstantin when applying would you 
please drop me or shall I resend without them?

> +
>   		inc_rlimit_ucounts(new->ucounts, UCOUNT_RLIMIT_NPROC, 1);
> +	}
>   	rcu_assign_pointer(task->real_cred, new);
>   	rcu_assign_pointer(task->cred, new);
> -	if (new->user != old->user || new->user_ns != old->user_ns)
> +	if (new->user != old->user || new->user_ns != old->user_ns || !uid_eq(new->uid, old->uid) ) {
> +		trace_printk("decr rlim for ns: %u val: %ld\n", old->ucounts->ns->ns.inum,
> +			     atomic_long_read(&old->ucounts->ucount[UCOUNT_RLIMIT_NPROC]));
>   		dec_rlimit_ucounts(old->ucounts, UCOUNT_RLIMIT_NPROC, 1);
> +	}
>   	alter_cred_subscribers(old, -2);
> 
>   	/* send notifications */
> --
> 2.34.1
>