linux-kernel - Re: clock_nanosleep() task

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+ydwtrdNL_7AvZpmJ__06YUCVhSr7YPO8D=wzXhZiWp1-DVqg@mail.gmail.com>
Date:	Wed, 6 Feb 2013 14:01:21 +0200
From:	Tommi Rantala <tt.rantala@...il.com>
To:	Stanislaw Gruszka <sgruszka@...hat.com>
Cc:	Thomas Gleixner <tglx@...utronix.de>,
	Oleg Nesterov <oleg@...hat.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Dave Jones <davej@...hat.com>,
	John Stultz <john.stultz@...aro.org>
Subject: Re: clock_nanosleep() task_struct leak

2013/2/6 Stanislaw Gruszka <sgruszka@...hat.com>:
> Below is proposed fix. Error cases wasn't that bad since there are
> various limitations when timer could be fired (i.e. timer which
> already fired can not be fired again).
>
> Tommi, please check if patch really fixes the problem. I tested it
> with signal interrupt and timeout scenarios, but I don't know how
> to confirm if it fix the leak or not.

Hi, looks good, this patch fixes the leaks I'm seeing.

Without the patch, running the program from my earlier mail shows
task_struct count growing:

{ttrantal@...ki ~}> uname -r
3.8.0-rc6+
{ttrantal@...ki ~}> sudo grep task_struct /proc/slabinfo
task_struct           93    123   8880    3    8 : tunables    0    0
  0 : slabdata     41     41      0
{ttrantal@...ki ~}> for i in `seq 1000` ; do ./leak ; done
{ttrantal@...ki ~}> sudo grep task_struct /proc/slabinfo
task_struct         1089   1089   8880    3    8 : tunables    0    0
  0 : slabdata    363    363      0
{ttrantal@...ki ~}> for i in `seq 1000` ; do ./leak ; done
{ttrantal@...ki ~}> sudo grep task_struct /proc/slabinfo
task_struct         2088   2088   8880    3    8 : tunables    0    0
  0 : slabdata    696    696      0
{ttrantal@...ki ~}>

With the patch applied, the leak is gone:

{ttrantal@...ki ~}> sudo grep task_struct /proc/slabinfo
task_struct           92    108   8880    3    8 : tunables    0    0
  0 : slabdata     36     36      0
{ttrantal@...ki ~}> for i in `seq 1000` ; do ./leak ; done
{ttrantal@...ki ~}> sudo grep task_struct /proc/slabinfo
task_struct           92    108   8880    3    8 : tunables    0    0
  0 : slabdata     36     36      0
{ttrantal@...ki ~}> for i in `seq 1000` ; do ./leak ; done
{ttrantal@...ki ~}> sudo grep task_struct /proc/slabinfo
task_struct           92    108   8880    3    8 : tunables    0    0
  0 : slabdata     36     36      0
{ttrantal@...ki ~}>

Running Trinity with kmemleak enabled also resulted to a lot of
detected leaks, which are all gone now based on a quick run.

> diff --git a/kernel/posix-cpu-timers.c b/kernel/posix-cpu-timers.c
> index 125cb67..07a38b6 100644
> --- a/kernel/posix-cpu-timers.c
> +++ b/kernel/posix-cpu-timers.c
> @@ -1424,6 +1424,7 @@ static int do_cpu_nanosleep(const clockid_t which_clock, int flags,
>                                 /*
>                                  * Our timer fired and was reset.
>                                  */
> +                               posix_cpu_timer_del(&timer);
>                                 spin_unlock_irq(&timer.it_lock);
>                                 return 0;
>                         }
> @@ -1441,9 +1442,17 @@ static int do_cpu_nanosleep(const clockid_t which_clock, int flags,
>                  * We were interrupted by a signal.
>                  */
>                 sample_to_timespec(which_clock, timer.it.cpu.expires, rqtp);
> -               posix_cpu_timer_set(&timer, 0, &zero_it, it);
> +               error = posix_cpu_timer_set(&timer, 0, &zero_it, it);
> +               if (!error)
> +                       posix_cpu_timer_del(&timer);
>                 spin_unlock_irq(&timer.it_lock);
>
> +               while (error == TIMER_RETRY) {
> +                       spin_lock_irq(&timer.it_lock);
> +                       error = posix_cpu_timer_del(&timer);
> +                       spin_unlock_irq(&timer.it_lock);
> +               }
> +
>                 if ((it->it_value.tv_sec | it->it_value.tv_nsec) == 0) {
>                         /*
>                          * It actually did fire already.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/