Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1757988AbcKCO5h (ORCPT <rfc822;w@1wt.eu>);
        Thu, 3 Nov 2016 10:57:37 -0400
Received: from out01.mta.xmission.com ([166.70.13.231]:53880 "EHLO
        out01.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1757882AbcKCO5M (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Thu, 3 Nov 2016 10:57:12 -0400
From: ebiederm@xmission.com (Eric W. Biederman)
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Jakub Kicinski <kubakici@wp.pl>, linux-kernel@vger.kernel.org,
        Kees Cook <keescook@chromium.org>,
        Johannes Berg <johannes@sipsolutions.net>,
        Maciej =?utf-8?Q?=C5=BBenczykowski?= <zenczykowski@gmail.com>
References: <20161103053948.GA40936@dtor-ws>
Date: Thu, 03 Nov 2016 09:54:25 -0500
In-Reply-To: <20161103053948.GA40936@dtor-ws> (Dmitry Torokhov's message of
        "Wed, 2 Nov 2016 22:39:48 -0700")
Message-ID: <87a8dgvcke.fsf@xmission.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
X-XM-SPF: eid=1c2JRm-0002eL-FV;;;mid=<87a8dgvcke.fsf@xmission.com>;;;hst=in01.mta.xmission.com;;;ip=50.193.239.161;;;frm=ebiederm@xmission.com;;;spf=neutral
X-XM-AID: U2FsdGVkX18lTNKxq5ViMKQDG4KAZu9SEeHM50aLxMw=
X-SA-Exim-Connect-IP: 50.193.239.161
X-SA-Exim-Mail-From: ebiederm@xmission.com
X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP
        *  0.0 T_TM2_M_HEADER_IN_MSG BODY: No description available.
        *  1.2 LotsOfNums_01 BODY: Lots of long strings of numbers
        *  0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60%
        *      [score: 0.5000]
        * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC
        *      [sa06 1397; Body=1 Fuz1=1 Fuz2=1]
X-Spam-DCC: XMission; sa06 1397; Body=1 Fuz1=1 Fuz2=1 
X-Spam-Combo: *;Dmitry Torokhov <dmitry.torokhov@gmail.com>
X-Spam-Relay-Country: 
X-Spam-Timing: total 6013 ms - load_scoreonly_sql: 0.05 (0.0%),
        signal_user_changed: 3.4 (0.1%), b_tie_ro: 2.4 (0.0%), parse: 0.84 (0.0%),
        extract_message_metadata: 16 (0.3%), get_uri_detail_list: 1.65 (0.0%),
        tests_pri_-1000: 6 (0.1%), tests_pri_-950: 1.21 (0.0%), tests_pri_-900: 0.95
        (0.0%), tests_pri_-400: 26 (0.4%), check_bayes: 25 (0.4%), b_tokenize: 7
        (0.1%), b_tok_get_all: 9 (0.1%), b_comp_prob: 2.4 (0.0%), b_tok_touch_all:
        4.1 (0.1%), b_finish: 0.76 (0.0%), tests_pri_0: 211 (3.5%),
        check_dkim_signature: 0.50 (0.0%), check_dkim_adsp: 2.6 (0.0%),
        tests_pri_500: 5745 (95.5%), poll_dns_idle: 5738 (95.4%), rewrite_mail: 0.00
        (0.0%)
Subject: Re: [PATCH] userns: suppress kmemleak message
X-Spam-Flag: No
X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600)
X-SA-Exim-Scanned: Yes (on in01.mta.xmission.com)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 2473
Lines: 72

Dmitry Torokhov <dmitry.torokhov@gmail.com> writes:

> We do not ever intend to unregister "user" sysctl table, unfortunately
> it leads kmemleak to believe that we are leaking memory:

Sounds like an issue with kmemleak because we do retain references.

So no we don't intend to unregister the table.

As for the patch.

Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com>

I can't see the using kmemleak_not_leak is possibly good form.  I
would much rather have suggestions about constructs that won't confuse
kmemleak and won't need ugly annotations that serve no purpose but to
appease a tool.  Perhaps the user_header variable needs to be moved out
of user_namespace_sysctl_init.

Eric

> unreferenced object 0xffff8807383bfd48 (size 96):
>   comm "swapper/0", pid 1, jiffies 4294894636 (age 278.320s)
>   hex dump (first 32 bytes):
>     a0 b4 b0 ba ff ff ff ff 00 00 00 00 01 00 00 00  ................
>     01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<ffffffffb7de59e8>] kmemleak_alloc+0x28/0x50
>     [<ffffffffb676e2f6>] __kmalloc+0x206/0x5a0
>     [<ffffffffb69be2d3>] __register_sysctl_table+0xb3/0x1130
>     [<ffffffffb69bf36b>] register_sysctl+0x1b/0x20
>     [<ffffffffba840de1>] user_namespace_sysctl_init+0x17/0x4c
>     [<ffffffffb60022b7>] do_one_initcall+0xb7/0x2a0
>     [<ffffffffba7eb102>] kernel_init_freeable+0x597/0x636
>     [<ffffffffb7de0433>] kernel_init+0x13/0x140
>     [<ffffffffb7dfb36a>] ret_from_fork+0x2a/0x40t show
>     [<ffffffffffffffff>] 0xffffffffffffffff
>
> Let's annotate the pointer as kmemleak_not_leak() to suppress the
> kmemleak false positive.
>
> Reported-by: Jakub Kicinski <kubakici@wp.pl>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>
> This was only compiled; Jakub, could you give it a spin?
>
>  kernel/ucount.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/kernel/ucount.c b/kernel/ucount.c
> index 9d20d5d..07d69b2 100644
> --- a/kernel/ucount.c
> +++ b/kernel/ucount.c
> @@ -5,6 +5,7 @@
>   *  License.
>   */
>  
> +#include <linux/kmemleak.h>
>  #include <linux/stat.h>
>  #include <linux/sysctl.h>
>  #include <linux/slab.h>
> @@ -226,6 +227,7 @@ static __init int user_namespace_sysctl_init(void)
>  	 */
>  	user_header = register_sysctl("user", empty);
>  	BUG_ON(!user_header);
> +	kmemleak_not_leak(user_header);
>  	BUG_ON(!setup_userns_sysctls(&init_user_ns));
>  #endif
>  	return 0;
> -- 
> 2.8.0.rc3.226.g39d4020