Received: by 2002:a05:6358:11c7:b0:104:8066:f915 with SMTP id i7csp4898892rwl; Tue, 28 Mar 2023 12:58:38 -0700 (PDT) X-Google-Smtp-Source: AK7set+Wkt158PUqA0lgpb47kYaUQNZCeQkBbOtdPyOOHUaieCnFpPP/Q7v5Vw5JWihUnv5m19ZM X-Received: by 2002:a05:6a20:65a5:b0:da:82fa:5945 with SMTP id p37-20020a056a2065a500b000da82fa5945mr13445911pzh.41.1680033518088; Tue, 28 Mar 2023 12:58:38 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1680033518; cv=none; d=google.com; s=arc-20160816; b=DAFctYZA9EuadDM469WJ83Qpxj3bKdzQ5KvBnUKuY9BvvFg0vhVeL+IC8Ru97IXq9m hJQZaDlTLJqEckktlMuBKLLyojHbSwMxTLWpaVu9Y3tBFYrdxLfwQXvCN3dBaL290Roo hDuzyj+EQyK/oyqS42f+9tUv55yZhQ7AObmK3CEBYdE+buyTb48wlpre1398UfS1Ee1y yuWI0httxKOOnBC/FwqrcEtDh63hmpU4x72b4QhYaMRRNOxfT4ZgMbyHaobiCBl8c4/B BHhXFXgtz0J20oPDly5VaqVYI9o3nf8canc3P1CCI5B8jtb0omSECFOciTZPKMVTx62l +Z+A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:ui-outboundreport:content-language :content-transfer-encoding:in-reply-to:mime-version:user-agent:date :message-id:from:references:cc:to:subject:dkim-signature; bh=MWVlgfwGB7Yby/dbVpdeQVquEKcNLzb2BPOOjKIjWfw=; b=pkynr6/FKRhnUUc6zeugHfNaQqcnP32QTzbJkCtq5DFL0Dhg3aMc45DBHCZ32QSurI xPRZxdQULokU5AQmrrJ86BmuA0wMqnl6seDPyzn/Rde9XWy52baZZcK6MpPl2u24jU9u zLLptsvt6VcAm/c/cW3LXiGx3mMYhfMuui4vU2ATlV/CaRQtEP/lazU+Xm8kfOaW/nSd gOMP/px8OgOqsmv0zoWQI2E8ahnm22c8P7dw9F4RfuxCd6rGPajB/ON2uWnY8lzBzx06 PIa2OR2OixxNZ6Fs4cGkr+INm0l6DU8rt6cwPF0tGZk4TY+k587lFqgeg4egLu+ezndN vahg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmx.de header.s=s31663417 header.b=CNog0E48; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gmx.de Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id m16-20020a62f210000000b00627ed1bc5fdsi24360812pfh.153.2023.03.28.12.58.24; Tue, 28 Mar 2023 12:58:38 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@gmx.de header.s=s31663417 header.b=CNog0E48; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gmx.de Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229674AbjC1Tzc (ORCPT + 99 others); Tue, 28 Mar 2023 15:55:32 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38164 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229493AbjC1Tzb (ORCPT ); Tue, 28 Mar 2023 15:55:31 -0400 Received: from mout.gmx.net (mout.gmx.net [212.227.15.19]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 35E0E1FC3; Tue, 28 Mar 2023 12:55:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.de; s=s31663417; t=1680033316; i=w_armin@gmx.de; bh=MWVlgfwGB7Yby/dbVpdeQVquEKcNLzb2BPOOjKIjWfw=; h=X-UI-Sender-Class:Subject:To:Cc:References:From:Date:In-Reply-To; b=CNog0E48vW4SG9cgfubf3+dHVjlQVfBWvI5EvSljObyNKv7Sx8KKd7xOAA02AxPCI Yz/5/mQn7C22Il1FbU4iko9ltu/6AJNmQ7WM5eabG7pt1n3BJhezOod8AS/Ddj3ZS0 WabI4J9ZgYI1xrN5u45+XrE5RwTeq/Xs81KNb2BkL+Cgt/KsUe4wnV/BrbHlP4Uy4O h35BdDSjVkpXNHlc58+VcZDbJAFVCoqj1BXk7jpaLs/m/rDTwT9jew8MVOfDNaADjr XLdHP3GvHpgLPKvZeb8naUnJarPZpowTAphv/xqsT4djsEln1WQ02CPDJv61FXDjtb LSCi5xmlmLjQw== X-UI-Sender-Class: 724b4f7f-cbec-4199-ad4e-598c01a50d3a Received: from [141.30.226.129] ([141.30.226.129]) by mail.gmx.net (mrgmx005 [212.227.17.190]) with ESMTPSA (Nemesis) id 1MWASe-1pw5Ow0F10-00XbUK; Tue, 28 Mar 2023 21:55:16 +0200 Subject: Re: [BUG] systemd-devd triggers kernel memleak apparently in drivers/core/dd.c: driver_register() To: Mirsad Goran Todorovac , Greg Kroah-Hartman Cc: "Rafael J. Wysocki" , linux-kernel@vger.kernel.org, Hans de Goede , Mark Gross , platform-driver-x86@vger.kernel.org References: <5059b11b-8b6e-394b-338f-49e1339067fa@alu.unizg.hr> <542c13f5-4cdd-7750-f10a-ef64bb7e8faa@alu.unizg.hr> <16862c45-2ffd-a2f2-6719-020c5d515800@alu.unizg.hr> From: Armin Wolf Message-ID: <4f65a23f-4e04-f04f-e56b-230a38ac5ec4@gmx.de> Date: Tue, 28 Mar 2023 21:55:15 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: <16862c45-2ffd-a2f2-6719-020c5d515800@alu.unizg.hr> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: quoted-printable Content-Language: en-US X-Provags-ID: V03:K1:APq1JikTrfCZ94d0nCsljU2IMtuSsBsV/K/xaNIzzRXMXKe+VQv LuoXrTsAanpH34GOmq1454TfoP3vysDqPBRwy86kFszaO9AAyRp2oaYkJIAE+Xv2OVbm4k5 W38Napx3rsIv5fXdkRT5444XhqJfR61ApvgsxonMlUyIC8SLs7vszkFQkNh4pmm9Mj0RG2m N5LHQoIs2CQoKSK76mW4w== UI-OutboundReport: notjunk:1;M01:P0:bkgGkoW6+cQ=;5fI2bQo+jGZqPAieWJjpMmg0fkb N5tOEHVze2RBx5eUopEXoSeksyrTdA0/X+R4lf9g3mCjRM1anUUa5UHS/k9TCfhu9BfK/rbY2 5LUQSaNSL5YoDMIANWCD+mGAwrewQHVgElTGFWah58gnYeTtmbPVvRwwf95EwlKMkfhksRLfK xarUW0FD0T4yCcy0PfqGFQgJ26fOliPIk95ZH/7t8NgAdmUPq/lKjcvp8OsohIp1dHOR1Gfgr QfbX0z7qQUmF0Q+lvbsJwIMmejeTYl5brZwZkjC851TTOaARw8W9LFDy6dkDTawR4fxgkg71s F4dr+THu8ORiQKssXv81mB/jQf9GPiDfwwulL8IpspsfdJG7SEojY/8UYLkX/wsiFMfGGdxpZ OZ/Ez/jAfn+FmhFw+7x1Z0RGDvXEfEWv1FpWB+EwKlKih1x4fBoM4gGFHS25hAcFKhQsCTLGC JlE6H4N1hg7vF5ykN3OtqSLudHj3i87FsrPzFajlJM0wTMCiIWJC9PWPMkg7UcMf+eAXt/cze HCUhjM/cm9W+had2YVWDYSYiS6TU/QG9vlCTiXP5zJdZITb7O0tZjdZP74ov7qNz9Qj4NXuT6 HDhHFA9L0knjo2cPKHtY5w9MVqxffmPR0fQ2R6+v83pZKWQvdNAUwwoGjNjp6E84/3kl55SIN ZfvfeE7dW3uaUvXiR+z42o6GmUe6rv4kY/f+cNCe+4F8HMcxDdfIVrp56CV4Z5+TGltu3gn1d pztFwbGIi3bEjjI/od6kgzTd96RKBjdi9Opbj99n8puueXeTuyxCSniBcZO7EiN9wDNUUULRH G5w42RtT7Y3bQdAMStliWqmdjTUHQO82iZvwzZqBx2HdVc2DoOrwh4RkBaJIYuraCDaZylptf cwXHge0NelrX61IiSyTqSkB8iMIHsmUJaH1gGS3Nyx0vT4vCY7KBw4FwGppk3UT/+1LM5b07g YpLDrbP92qFP8DijNQdvNzNgib0= X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,NICE_REPLY_A, RCVD_IN_DNSWL_LOW,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Am 28.03.23 um 21:06 schrieb Mirsad Goran Todorovac: > On 3/28/2023 6:53 PM, Armin Wolf wrote: >> Am 28.03.23 um 14:44 schrieb Mirsad Todorovac: >> >>> On 3/28/23 14:17, Greg Kroah-Hartman wrote: >>>> On Tue, Mar 28, 2023 at 02:08:06PM +0200, Mirsad Todorovac wrote: >>>>> On 3/28/23 13:59, Mirsad Todorovac wrote: >>>>> >>>>>> On 3/28/23 13:28, Greg Kroah-Hartman wrote: >>>>>>> On Tue, Mar 28, 2023 at 01:13:33PM +0200, Mirsad Todorovac wrote: >>>>>>>> Hi all, >>>>>>>> >>>>>>>> Here is another kernel memory leak report, just as I thought we >>>>>>>> have done with >>>>>>>> them by the xhci patch by Mathias. >>>>>>>> >>>>>>>> The memory leaks were caught on an AlmaLinux 8.7 (CentOS) fork >>>>>>>> system, running >>>>>>>> on a Lenovo desktop box (see lshw.txt) and the newest Linux >>>>>>>> kernel 6.3-rc4 commit >>>>>>>> g3a93e40326c8 with Mathias' patch for a xhci systemd-devd >>>>>>>> triggered leak. >>>>>>>> >>>>>>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 See: >>>>>>>> <20230327095019.1017159-1-mathias.nyman@linux.intel.com> on LKML. >>>>>>>> >>>>>>>> This leak is also systemd-devd triggered, except for the >>>>>>>> memstick_check() leaks >>>>>>>> which I was unable to bisect due to the box not booting older >>>>>>>> kernels (work in >>>>>>>> progress). >>>>>>>> >>>>>>>> unreferenced object 0xffff88ad12392710 (size 96): >>>>>>>> =C2=A0=C2=A0=C2=A0 comm "systemd-udevd", pid 735, jiffies 4294896= 759 (age >>>>>>>> 2257.568s) >>>>>>>> =C2=A0=C2=A0=C2=A0 hex dump (first 32 bytes): >>>>>>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 53 65 72 69 61 6c 50 6f 72 74 31 4= 1 64 64 72 65 >>>>>>>> SerialPort1Addre >>>>>>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 73 73 2c 33 46 38 2f 49 52 51 34 3= b 5b 4f 70 74 >>>>>>>> ss,3F8/IRQ4;[Opt >>>>>>>> =C2=A0=C2=A0=C2=A0 backtrace: >>>>>>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 [] slab_post_all= oc_hook+0x8c/0x3e0 >>>>>>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 [] __kmem_cache_= alloc_node+0x1d9/0x2a0 >>>>>>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 [] __kmalloc_nod= e_track_caller+0x59/0x180 >>>>>>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 [] kstrdup+0x3a/= 0x70 >>>>>>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 [] >>>>>>>> tlmi_extract_output_string.isra.0+0x2a/0x60 [think_lmi] >>>>>>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 [] tlmi_setting.= constprop.4+0x54/0x90 >>>>>>>> [think_lmi] >>>>>>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 [] tlmi_probe+0x= 591/0xba0 [think_lmi] >>>>>>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 [] wmi_dev_probe= +0x163/0x230 [wmi] >>>>>>> >> Hi, >> >> this "SerialPort1Address" string looks like a BIOS setup option, and >> indeed think_lmi allows for >> changing BIOS setup options over sysfs. While looking at >> current_value_show() in think-lmi.c, i noticed >> that "item" holds a string which is allocated with kstrdup(), so it >> has to be freed using kfree(). >> This however does not happen if strbrk() fails, so maybe the memory >> leak is caused by this? >> >> Armin Wolf > > Hi Armin, > > I tried your suggestion, and though it is an obvious improvement and a > leak fix, this > was not the one we were searching for. > > I tested the following patch: > > diff --git a/drivers/platform/x86/think-lmi.c > b/drivers/platform/x86/think-lmi.c > index c816646eb661..1e77ecb0cba8 100644 > --- a/drivers/platform/x86/think-lmi.c > +++ b/drivers/platform/x86/think-lmi.c > @@ -929,8 +929,10 @@ static ssize_t current_value_show(struct kobject > *kobj, struct kobj_attribute *a > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 /* validate and split from `i= tem,value` -> `value` */ > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 value =3D strpbrk(item, ","); > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (!value || value =3D=3D item ||= !strlen(value + 1)) > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (!value || value =3D=3D item ||= !strlen(value + 1)) { > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0 kfree(item); > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 return -EINVAL; > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 } > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ret =3D sysfs_emit(buf, "%s\n= ", value + 1); > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 kfree(item); > > (I would also object to the use of strlen() here, for it is inherently > insecure > against SEGFAULT in kernel space.) > > I still get: > [root@pc-mtodorov marvin]# uname -rms > Linux 6.3.0-rc4-armin-patch-00025-g3a93e40326c8-dirty x86_64 > [root@pc-mtodorov marvin]# cat /sys/kernel/debug/kmemleak [edited] > unreferenced object 0xffff8eb008ef9260 (size 96): > =C2=A0 comm "systemd-udevd", pid 771, jiffies 4294896499 (age 74.880s) > =C2=A0 hex dump (first 32 bytes): > =C2=A0=C2=A0=C2=A0 53 65 72 69 61 6c 50 6f 72 74 31 41 64 64 72 65 Seria= lPort1Addre > =C2=A0=C2=A0=C2=A0 73 73 2c 33 46 38 2f 49 52 51 34 3b 5b 4f 70 74 ss,3F= 8/IRQ4;[Opt > =C2=A0 backtrace: > =C2=A0=C2=A0=C2=A0 [] slab_post_alloc_hook+0x8c/0x3e0 > =C2=A0=C2=A0=C2=A0 [] __kmem_cache_alloc_node+0x1d9/0x= 2a0 > =C2=A0=C2=A0=C2=A0 [] __kmalloc_node_track_caller+0x59= /0x180 > =C2=A0=C2=A0=C2=A0 [] kstrdup+0x3a/0x70 > =C2=A0=C2=A0=C2=A0 [] tlmi_extract_output_string.isra.= 0+0x2a/0x60 > [think_lmi] > =C2=A0=C2=A0=C2=A0 [] tlmi_setting.constprop.4+0x54/0x= 90 [think_lmi] > =C2=A0=C2=A0=C2=A0 [] tlmi_probe+0x591/0xba0 [think_lm= i] > =C2=A0=C2=A0=C2=A0 [] wmi_dev_probe+0x163/0x230 [wmi] > =C2=A0=C2=A0=C2=A0 [] really_probe+0x17b/0x3d0 > =C2=A0=C2=A0=C2=A0 [] __driver_probe_device+0x84/0x190 > =C2=A0=C2=A0=C2=A0 [] driver_probe_device+0x24/0xc0 > =C2=A0=C2=A0=C2=A0 [] __driver_attach+0xc2/0x190 > =C2=A0=C2=A0=C2=A0 [] bus_for_each_dev+0x81/0xd0 > =C2=A0=C2=A0=C2=A0 [] driver_attach+0x22/0x30 > =C2=A0=C2=A0=C2=A0 [] bus_add_driver+0x1b4/0x240 > =C2=A0=C2=A0=C2=A0 [] driver_register+0x62/0x120 > unreferenced object 0xffff8eb018ddbb40 (size 64): > =C2=A0 comm "systemd-udevd", pid 771, jiffies 4294896528 (age 74.780s) > =C2=A0 hex dump (first 32 bytes): > =C2=A0=C2=A0=C2=A0 55 53 42 50 6f 72 74 41 63 63 65 73 73 2c 45 6e USBPo= rtAccess,En > =C2=A0=C2=A0=C2=A0 61 62 6c 65 64 3b 5b 4f 70 74 69 6f 6e 61 6c 3a abled= ;[Optional: > =C2=A0 backtrace: > =C2=A0=C2=A0=C2=A0 [] slab_post_alloc_hook+0x8c/0x3e0 > =C2=A0=C2=A0=C2=A0 [] __kmem_cache_alloc_node+0x1d9/0x= 2a0 > =C2=A0=C2=A0=C2=A0 [] __kmalloc_node_track_caller+0x59= /0x180 > =C2=A0=C2=A0=C2=A0 [] kstrdup+0x3a/0x70 > =C2=A0=C2=A0=C2=A0 [] tlmi_extract_output_string.isra.= 0+0x2a/0x60 > [think_lmi] > =C2=A0=C2=A0=C2=A0 [] tlmi_setting.constprop.4+0x54/0x= 90 [think_lmi] > =C2=A0=C2=A0=C2=A0 [] tlmi_probe+0x591/0xba0 [think_lm= i] > =C2=A0=C2=A0=C2=A0 [] wmi_dev_probe+0x163/0x230 [wmi] > =C2=A0=C2=A0=C2=A0 [] really_probe+0x17b/0x3d0 > =C2=A0=C2=A0=C2=A0 [] __driver_probe_device+0x84/0x190 > =C2=A0=C2=A0=C2=A0 [] driver_probe_device+0x24/0xc0 > =C2=A0=C2=A0=C2=A0 [] __driver_attach+0xc2/0x190 > =C2=A0=C2=A0=C2=A0 [] bus_for_each_dev+0x81/0xd0 > =C2=A0=C2=A0=C2=A0 [] driver_attach+0x22/0x30 > =C2=A0=C2=A0=C2=A0 [] bus_add_driver+0x1b4/0x240 > =C2=A0=C2=A0=C2=A0 [] driver_register+0x62/0x120 > unreferenced object 0xffff8eb006fe2b40 (size 64): > =C2=A0 comm "systemd-udevd", pid 771, jiffies 4294896542 (age 74.724s) > =C2=A0 hex dump (first 32 bytes): > =C2=A0=C2=A0=C2=A0 55 53 42 42 49 4f 53 53 75 70 70 6f 72 74 2c 45 USBBI= OSSupport,E > =C2=A0=C2=A0=C2=A0 6e 61 62 6c 65 64 3b 5b 4f 70 74 69 6f 6e 61 6c nable= d;[Optional > =C2=A0 backtrace: > =C2=A0=C2=A0=C2=A0 [] slab_post_alloc_hook+0x8c/0x3e0 > =C2=A0=C2=A0=C2=A0 [] __kmem_cache_alloc_node+0x1d9/0x= 2a0 > =C2=A0=C2=A0=C2=A0 [] __kmalloc_node_track_caller+0x59= /0x180 > =C2=A0=C2=A0=C2=A0 [] kstrdup+0x3a/0x70 > =C2=A0=C2=A0=C2=A0 [] tlmi_extract_output_string.isra.= 0+0x2a/0x60 > [think_lmi] > =C2=A0=C2=A0=C2=A0 [] tlmi_setting.constprop.4+0x54/0x= 90 [think_lmi] > =C2=A0=C2=A0=C2=A0 [] tlmi_probe+0x591/0xba0 [think_lm= i] > =C2=A0=C2=A0=C2=A0 [] wmi_dev_probe+0x163/0x230 [wmi] > =C2=A0=C2=A0=C2=A0 [] really_probe+0x17b/0x3d0 > =C2=A0=C2=A0=C2=A0 [] __driver_probe_device+0x84/0x190 > =C2=A0=C2=A0=C2=A0 [] driver_probe_device+0x24/0xc0 > =C2=A0=C2=A0=C2=A0 [] __driver_attach+0xc2/0x190 > =C2=A0=C2=A0=C2=A0 [] bus_for_each_dev+0x81/0xd0 > =C2=A0=C2=A0=C2=A0 [] driver_attach+0x22/0x30 > =C2=A0=C2=A0=C2=A0 [] bus_add_driver+0x1b4/0x240 > =C2=A0=C2=A0=C2=A0 [] driver_register+0x62/0x120 > > There are currently 84 wmi_dev_probe leaks, sized mostly 64 bytes, and > one 96 and two 192 bytes. > > I also cannot figure out the mechanism by which current_value_show() > is called, when it is static? > > Any idea? > > Thanks. > > Best regards, > Mirsad > Can you tell me how many BIOS settings think-lmi provides on your machine?= Because according to the stacktrace, the other place where the leak could have occurred is inside tlmi_analyze(= ), which calls tlmi_setting(). However, i have no idea on how *info is somehow leaked, it has to happen i= nside the for-loop between the call to tlmi_setting() and strreplace(), because otherwise the strings would no= t contain the "/" character. Can you check if the problem is somehow solved by applying the following c= ommit from the platform-drivers-x86 for-next branch: da62908efe80 ("platform/x86: think-lmi: Properly interpret return value of= tlmi_setting") Also current_value_show() is used by attr_current_val, the __ATTR_RW_MODE(= ) macro arranges for that. Armin Wolf >>>>>>> Why aren't you looking at the wmi.c driver?=C2=A0 That should be >>>>>>> where the >>>>>>> issue is, not the driver core, right? >>>>>>> >>>>>>> thanks, >>>>>>> >>>>>>> greg k-h >>>>>> >>>>>> Hi, Mr. Greg, >>>>>> >>>>>> Thanks for the quick reply. >>>>>> >>>>>> I have added CC: for additional developers per >>>>>> drivers/platform/x86/wmi.c, >>>>>> however, this seems to me like hieroglyphs. There is nothing >>>>>> obvious, but >>>>>> I had not noticed it with v6.3-rc3? >>>>>> >>>>>> Maybe, there seems to be something off: >>>>>> >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 949 static int wmi_dev_probe(struct devic= e *dev) >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 950 { >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 951=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0 struct wmi_block *wblock =3D dev_to_wblock(dev); >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 952=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0 struct wmi_driver *wdriver =3D >>>>>> drv_to_wdrv(dev->driver); >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 953=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0 int ret =3D 0; >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 954=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0 char *buf; >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 955 >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 956=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0 if (ACPI_FAILURE(wmi_method_enable(wblock, true))) >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 957=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 dev_warn(dev,= "failed to enable device >>>>>> -- probing anyway\n"); >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 958 >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 959=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0 if (wdriver->probe) { >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 960=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ret =3D wdriv= er->probe(dev_to_wdev(dev), >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 961 find_guid_context(wblock, wdriver)); >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 962=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (ret !=3D = 0) >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 963=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 goto probe_failure; >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 964=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0 } >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 965 >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 966=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0 /* driver wants a character device made */ >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 967=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0 if (wdriver->filter_callback) { >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 968=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 /* check that= required buffer size >>>>>> declared by driver or MOF */ >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 969=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (!wblock->= req_buf_size) { >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 970 dev_err(&wblock->dev.dev, >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 971=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0 "Required buffer size >>>>>> not set\n"); >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 972=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ret =3D -EINVAL; >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 973=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 goto probe_failure; >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 974=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 } >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 975 >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 976=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 wblock->handl= er_data =3D >>>>>> kmalloc(wblock->req_buf_size, >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 977 GFP_KERNEL); >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 978=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (!wblock->= handler_data) { >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 979=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ret =3D -ENOMEM; >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 980=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 goto probe_failure; >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 981=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 } >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 982 >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 983=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 buf =3D kaspr= intf(GFP_KERNEL, "wmi/%s", >>>>>> wdriver->driver.name); >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 984=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (!buf) { >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 985=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ret =3D -ENOMEM; >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 986=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 goto probe_string_failure; >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 987=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 } >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 988=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 wblock->char_= dev.minor =3D >>>>>> MISC_DYNAMIC_MINOR; >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 989=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 wblock->char_= dev.name =3D buf; >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 990=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 wblock->char_= dev.fops =3D &wmi_fops; >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 991=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 wblock->char_= dev.mode =3D 0444; >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 992=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ret =3D misc_= register(&wblock->char_dev); >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 993=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (ret) { >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 994=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 dev_warn(dev, "failed to >>>>>> register char dev: %d\n", ret); >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 995=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ret =3D -ENOMEM; >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 996=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 goto probe_misc_failure; >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 997=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 } >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 998=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0 } >>>>>> =C2=A0 =C2=A0=C2=A0=C2=A0 999 >>>>>> =C2=A0 =C2=A0=C2=A0 1000=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0 set_bit(WMI_PROBED, &wblock->flags); >>>>>> =C2=A0 =C2=A0=C2=A0 1001=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0 return 0; >>>>>> =C2=A0 =C2=A0=C2=A0 1002 >>>>>> =C2=A0 =C2=A0=C2=A0 1003 probe_misc_failure: >>>>>> =C2=A0 =C2=A0=C2=A0 1004=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0 kfree(buf); >>>>>> =C2=A0 =C2=A0=C2=A0 1005 probe_string_failure: >>>>>> =C2=A0 =C2=A0=C2=A0 1006=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0 kfree(wblock->handler_data); >>>>>> =C2=A0 =C2=A0=C2=A0 1007 probe_failure: >>>>>> =C2=A0 =C2=A0=C2=A0 1008=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0 if (ACPI_FAILURE(wmi_method_enable(wblock, >>>>>> false))) >>>>>> =C2=A0 =C2=A0=C2=A0 1009=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 dev_warn(dev, "fail= ed to disable >>>>>> device\n"); >>>>>> >>>>>> >>>>>> char *buf is passed to kfree(buf) uninitialised if >>>>>> wdriver->filter_callback >>>>>> is not set. >>>>>> >>>>>> It seems like a logical error per se, but I don't believe this is >>>>>> the cause >>>>>> of the leak? >>>>> >>>>> CORRECTION: >>>>> >>>>> I overlooked the "return 0" in line 1001. >>>> >>>> Yeah, and the memory looks to be freed properly in the >>>> wmi_dev_remove() >>>> callback, right? >>> >>> It would appear so. To verify that: >>> >>> Alloc: >>> 976=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 wblock->handler_data =3D= kmalloc(wblock->req_buf_size, >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0 GFP_KERNEL); >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 >>> >>> 983=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 buf =3D kasprintf(GFP_KE= RNEL, "wmi/%s", wdriver->driver.name); >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 >>> 989=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 wblock->char_dev.name = =3D buf; >>> >>> In lines 1022-1023: >>> >>> 1022=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 kfree(wblock->char_dev.= name); >>> 1023=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 kfree(wblock->handler_d= ata); >>> >>>>> This is why I don't think things should be rushed, but analysed >>>>> with clear and >>>>> cold head. And with as many eyes as possible :) >>>>> >>>>> The driver stuff is my long-term research interest. To state the >>>>> obvious, >>>>> the printing and multimedia education and industry in general >>>>> would benefit from >>>>> the open-source drivers for many instruments that still work, but >>>>> are obsoleted >>>>> by the producer and require unsupported versions of the OS. >>>>> >>>>> Thank you again for reviewing the bug report, however, ATM I do >>>>> not think I have >>>>> what it takes to hunt down the memleak. :-/ >>>> >>>> Do you have a reproducer that you can use to show the problem better? >>> >>> Unfortunately, the problem doesn't seem to appear during the run of >>> a particular >>> test, but immediately on startup of the OS. This makes it awkward to >>> pinpoint the >>> exact service that triggered memory leaks. But they would appear to >>> have to do >>> with the initialisation of the USB devices, wouldn't they? >>> >>> There seem to be strings: >>> >>> "USBPortAccess,Enabled;[Optional:" >>> "USBBIOSSupport,Enabled;[Optional" >>> "USBEnumerationDelay,Disabled;[Op" >>> >>> This seems to be happening during USB initialisation and before any >>> services. >>> But I might as well be wrong. >>> >>>> Or can you test kernel patches to verify the problem is fixed or >>>> not if >>>> we send you patches to test? >>> >>> Certainly, Lord willing, I can test the patches in the same >>> environment that >>> mainfeted the bug (or memleak). >>> >>> Best regards, >>> Mirsad >>> >