Re: [PATCH v2 3/5] exec: Move cleanup of posix timers on exec out of de_thread

Christian Brauner
2020-03-09T19:59:54Z

On Sun, Mar 08, 2020 at 04:36:55PM -0500, Eric W. Biederman wrote:
>
> These functions have very little to do with de_thread move them out
> of de_thread an into flush_old_exec proper so it can be more clearly
> seen what flush_old_exec is doing.
>
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> ---
> fs/exec.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/fs/exec.c b/fs/exec.c
> index ff74b9a74d34..215d86f77b63 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -1189,11 +1189,6 @@ static int de_thread(struct task_struct *tsk)

While you're cleaning up de_thread() wouldn't it be good to also take
the opportunity and remove the task argument from de_thread(). It's only
ever used with current. Could be done in one of your patches or as a
separate patch.

diff --git a/fs/exec.c b/fs/exec.c
index db17be51b112..ee108707e4b0 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1061,8 +1061,9 @@ static int exec_mmap(struct mm_struct *mm)
* disturbing other processes. (Other processes might share the signal
* table via the CLONE_SIGHAND option to clone().)
*/
-static int de_thread(struct task_struct *tsk)
+static int de_thread(void)
{
+ struct task_struct *tsk = current;
struct signal_struct *sig = tsk->signal;
struct sighand_struct *oldsighand = tsk->sighand;
spinlock_t *lock = &oldsighand->siglock;
@@ -1266,7 +1267,7 @@ int flush_old_exec(struct linux_binprm * bprm)
* Make sure we have a private signal table and that
* we are unassociated from the previous thread group.
*/
- retval = de_thread(current);
+ retval = de_thread();
if (retval)
goto out;