linux-kernel - Re: [PATCH] introduce a default off mode for kmemleak

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: <1279054932.16438.3.camel@e102109-lin.cambridge.arm.com>
Date:	Tue, 13 Jul 2010 22:02:12 +0100
From:	Catalin Marinas <catalin.marinas@....com>
To:	Jason Baron <jbaron@...hat.com>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] introduce a default off mode for kmemleak

Hi Jason,

On Tue, 2010-07-13 at 21:31 +0100, Jason Baron wrote:
> Introduce a new DEBUG_KMEMLEAK_DEFAULT_OFF config parameter that allows
> kmemleak to be disabled by default, but enabled on the command line via:
> kmemleak=on. Although a reboot is required to turn it on, its still
> useful to not require a re-compile.
> 
> Signed-off-by: Jason Baron <jbaron@...hat.com>

The patch makes sense. I have a few comments below.

> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> index 2c0d032..1006267 100644
> --- a/mm/kmemleak.c
> +++ b/mm/kmemleak.c
> @@ -211,6 +211,11 @@ static signed long jiffies_scan_wait;
>  static int kmemleak_stack_scan = 1;
>  /* protects the memory scanning, parameters and debug/kmemleak file access */
>  static DEFINE_MUTEX(scan_mutex);
> +#ifdef CONFIG_DEBUG_KMEMLEAK_DEFAULT_OFF
> +/* setting kmemleak=on, will set this var, skipping the disable */
> +static int kmemleak_skip_disable;
> +#endif

Can we have kmemleak_skip_disable always and remove the #ifdef?

> @@ -1600,10 +1605,22 @@ static int kmemleak_boot_config(char *str)
>  {
>         if (!str)
>                 return -EINVAL;
> +
>         if (strcmp(str, "off") == 0)
> +#ifdef CONFIG_DEBUG_KMEMLEAK_DEFAULT_OFF
> +               return -EINVAL;
> +#else
>                 kmemleak_disable();
> -       else if (strcmp(str, "on") != 0)
> +#endif
> +       else if (strcmp(str, "on") == 0)
> +#ifdef CONFIG_DEBUG_KMEMLEAK_DEFAULT_OFF
> +               kmemleak_skip_disable = 1;
> +#else
> +               return -EINVAL;
> +#endif
> +       else
>                 return -EINVAL;
> +
>         return 0;
>  }

Here I would remove all these #ifdef's. It doesn't really matter if you
pass kmemleak=off/on on the command line and kmemleak was already
disable/enabled. If we remove the #ifdef for the kmemleak_skip_disable,
we can always set the variable here.

> @@ -1616,6 +1633,13 @@ void __init kmemleak_init(void)
>         int i;
>         unsigned long flags;
> 
> +#ifdef CONFIG_DEBUG_KMEMLEAK_DEFAULT_OFF
> +       if (!kmemleak_skip_disable) {
> +               kmemleak_disable();
> +               return;
> +       }
> +#endif

That's the only place where we could keep the #ifdef.

Thanks.

-- 
Catalin

--
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/