Project

General

Profile

Actions
Copy link

Bug #13622

closed

Memory leak in coretemp_create_sensor

Added by Yuri Pankov 2 months ago. Updated about 2 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Robert Mustacchi
Category:
driver - device drivers
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:
1346

Description

> ::findleaks
CACHE             LEAKED           BUFCTL CALLER
...
fffffdea17cb1000       1 fffffdf063c267c8 coretemp_create_sensor+0x44
fffffdea17cb1000       1 fffffdf0195a6c50 coretemp_create_sensor+0x44
fffffdea17cb1000       2 fffffdf02c193878 coretemp_create_sensor+0x44
fffffdea17cb1000      22 fffffdf063c25100 coretemp_create_sensor+0x44
...
------------------------------------------------------------------------
          Total     229 buffers, 221224 bytes

> fffffdf063c267c8::bufctl -v
           ADDR          BUFADDR        TIMESTAMP           THREAD
                           CACHE          LASTLOG         CONTENTS
fffffdf063c267c8 fffffdec473bb8d8       20da21084e fffffbc3bec1ac20
                fffffdea17cb1000 fffffdecbb73f040                0
                kmem_cache_alloc_debug+0x2fc
                kmem_cache_alloc+0x273
                kmem_zalloc+0x47
                coretemp_create_sensor+0x44
                coretemp_walk+0xca
                cmi_hdl_walk+0x93
                coretemp_attach+0xcc
                devi_attach+0xa1
                attach_node+0x8b
                i_ndi_config_node+0x95
                i_ddi_attachchild+0x3a
                devi_attach_node+0x5d
                config_immediate_children+0xd0
                devi_config_common+0x69
                mt_config_thread+0x13a

The reason is simple -- coretemp_create_sensor() forgot to actually add the created sensor to the list, so cleanup in coretemp_destroy() did not have anything to remove/free:

diff --git a/usr/src/uts/intel/io/coretemp/coretemp.c b/usr/src/uts/intel/io/coretemp/coretemp.c
index bea8078002..1021f4eb4c 100644
--- a/usr/src/uts/intel/io/coretemp/coretemp.c
+++ b/usr/src/uts/intel/io/coretemp/coretemp.c
@@ -389,6 +389,7 @@ coretemp_create_sensor(coretemp_t *ct, cmi_hdl_t hdl, uint_t tjmax,
                    "for %s: %d", sensor->cs_name, err);
        }

+       list_insert_tail(&ct->coretemp_sensors, sensor);
        return (B_TRUE);
 err:
        kmem_free(sensor, sizeof (coretemp_sensor_t));

Actions
Copy link
 #1

Updated by Robert Mustacchi 2 months ago

  • Assignee set to Robert Mustacchi

Thanks for writing this up, Yuri. I agree with the analysis here. I'll test this out and verify that we've cleaned up the leak.

Actions
Copy link
 #2

Updated by Electric Monk about 2 months ago

  • Gerrit CR set to 1346
Actions
Copy link
 #3

Updated by Robert Mustacchi about 2 months ago

We've tested this (ht Patrick) by loading this up on an Intel system with coretemp and verifying that after doing a rem_drv, nothing showed up in topo, and then panicked the system. From the dump there was no memory leak due to coretemp.

Actions
Copy link
 #4

Updated by Electric Monk about 2 months ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100

git commit 173f6047c6877d03cbb55428e6ec95f07c9cbb83

commit  173f6047c6877d03cbb55428e6ec95f07c9cbb83
Author: Robert Mustacchi <rm@fingolfin.org>
Date:   2021-03-18T17:44:49.000Z

    13622 Memory leak in coretemp_create_sensor
    Reviewed by: Yuri Pankov <yuripv@yuripv.dev>
    Reviewed by: Jason King <jason.king@joyent.com>
    Reviewed by: Paul Winder <paul@winder.uk.net>
    Reviewed by: Toomas Soome <tsoome@me.com>
    Approved by: Dan McDonald <danmcd@joyent.com>

Actions
Copy link

Also available in: Atom PDF