linux-kernel - Re: [PATCH] exit: call add_device_randomness() without tasklist

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: <CAGudoHFYyQNW8cyrG9aj-vmRFbKAkrcAf_SyB5aNzmx3a73eBg@mail.gmail.com>
Date: Tue, 28 Jan 2025 16:23:11 +0100
From: Mateusz Guzik <mjguzik@...il.com>
To: Oleg Nesterov <oleg@...hat.com>
Cc: brauner@...nel.org, akpm@...ux-foundation.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] exit: call add_device_randomness() without tasklist_lock

On Tue, Jan 28, 2025 at 4:21 PM Oleg Nesterov <oleg@...hat.com> wrote:
>
> On 01/28, Mateusz Guzik wrote:
> >
> > --- a/kernel/exit.c
> > +++ b/kernel/exit.c
> > @@ -141,13 +141,14 @@ static void __unhash_process(struct task_struct *p, bool group_dead)
> >  /*
> >   * This function expects the tasklist_lock write-locked.
> >   */
> > -static void __exit_signal(struct task_struct *tsk)
> > +static unsigned long long __exit_signal(struct task_struct *tsk)
> >  {
> >       struct signal_struct *sig = tsk->signal;
> >       bool group_dead = thread_group_leader(tsk);
> >       struct sighand_struct *sighand;
> >       struct tty_struct *tty;
> >       u64 utime, stime;
> > +     unsigned long long randomness;
> >
> >       sighand = rcu_dereference_check(tsk->sighand,
> >                                       lockdep_tasklist_lock_is_held());
> > @@ -174,8 +175,7 @@ static void __exit_signal(struct task_struct *tsk)
> >                       sig->curr_target = next_thread(tsk);
> >       }
> >
> > -     add_device_randomness((const void*) &tsk->se.sum_exec_runtime,
> > -                           sizeof(unsigned long long));
> > +     randomness = tsk->se.sum_exec_runtime;
> >
> >       /*
> >        * Accumulate here the counters for all threads as they die. We could
> > @@ -214,6 +214,8 @@ static void __exit_signal(struct task_struct *tsk)
> >               flush_sigqueue(&sig->shared_pending);
> >               tty_kref_put(tty);
> >       }
> > +
> > +     return randomness;
> >  }
> >
> >  static void delayed_put_task_struct(struct rcu_head *rhp)
> > @@ -242,6 +244,7 @@ void release_task(struct task_struct *p)
> >       struct task_struct *leader;
> >       struct pid *thread_pid;
> >       int zap_leader;
> > +     unsigned long long randomness;
> >  repeat:
> >       /* don't need to get the RCU readlock here - the process is dead and
> >        * can't be modifying its own credentials. But shut RCU-lockdep up */
> > @@ -254,7 +257,7 @@ void release_task(struct task_struct *p)
> >       write_lock_irq(&tasklist_lock);
> >       ptrace_release_task(p);
> >       thread_pid = get_pid(p->thread_pid);
> > -     __exit_signal(p);
> > +     randomness = __exit_signal(p);
> >
> >       /*
> >        * If we are the last non-leader member of the thread
> > @@ -280,6 +283,8 @@ void release_task(struct task_struct *p)
> >       put_pid(thread_pid);
> >       release_thread(p);
> >       put_task_struct_rcu_user(p);
> > +     add_device_randomness((const void*) &randomness,
> > +                           sizeof(unsigned long long));
> >
> >       p = leader;
> >       if (unlikely(zap_leader))
>
> Hmm, why not simply shift add_device_randomness(sum_exec_runtime) to
> release_release_task() ?
>

I wanted to keep the load where it was, but see my follow up. :)


-- 
Mateusz Guzik <mjguzik gmail.com>