Project

General

Profile

Bug #671

libzfs_fru_gather can leak string - 7003347

Added by Damian Wojslaw over 9 years ago. Updated over 9 years ago.

Status:
In Progress
Priority:
Low
Category:
lib - userland libraries
Start date:
2011-01-26
Due date:
% Done:

0%

Estimated time:
Difficulty:
Tags:
Gerrit CR:

Description

http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=7003347

Below is a paste from original bug report. I'm filling it in and will check to see if this condition affects illumos also. If so, I will prepare a patch.


Noticed the following leak in fmd:

umem_alloc_16 leak: 1 buffer, 16 bytes
            ADDR          BUFADDR        TIMESTAMP           THREAD
                            CACHE          LASTLOG         CONTENTS
        152f53d0         152f6060    50178561d2963               12
                          82bc010                0                0
                 libumem.so.1`umem_cache_alloc_debug+0x144
                 libumem.so.1`umem_cache_alloc+0x19a
                 libumem.so.1`umem_alloc+0xcd
                 libtopo.so.1`topo_alloc+0x21
                 libtopo.so.1`topo_hdl_alloc+0x19
                 libtopo.so.1`topo_hdl_strdup+0x2e
                 libtopo.so.1`prop_getval+0x611
                 libtopo.so.1`topo_prop_get_string+0x1e
                 libzfs.so.1`libzfs_fru_gather+0x6f
                 libtopo.so.1`topo_walk_step+0x68
                 libtopo.so.1`step_sibling+0x73
                 libtopo.so.1`topo_walk_step+0xe1
                 libzfs.so.1`libzfs_fru_refresh+0xdc
                 libzfs.so.1`libzfs_fru_compare+0x28
                 zfs-retire.so`find_vdev+0x48

At the beginning of libzfs_fru_gather(), we have this code:

        /*
         * If this is the chassis node, and we don't yet have the system
         * chassis ID, then fill in this value now.
         */
        if (hdl->libzfs_chassis_id[0] == '\0' &&
            strcmp(_topo_node_name(tn), "chassis") == 0) {
                if (_topo_prop_get_string(tn, FM_FMRI_AUTHORITY,
                    FM_FMRI_AUTH_CHASSIS, &devpath, &err) == 0)
                        (void) strlcpy(hdl->libzfs_chassis_id, devpath,
                            sizeof (hdl->libzfs_chassis_id));
        }

If we entered the above if clause, then devpath contains a dynamically allocated string that needs to be free'd.  However, before we do that, we replace it with another string

. . .

        /*
         * Get the devfs path and FRU.
         */
        if (_topo_prop_get_string(tn, "io", "devfs-path", &devpath, &err) != 0)
                return (TOPO_WALK_NEXT);

Looks like the simple fix is to add a _topo_hdl_strfree() call right after the strlcpy above.

History

#1

Updated by Damian Wojslaw over 9 years ago

  • Status changed from New to In Progress
#2

Updated by Damian Wojslaw over 9 years ago

bash-4.0$ hg pdiff

diff -r e02b6d3917e4 -r fffcee42ff30 usr/src/lib/libzfs/common/libzfs_fru.c
--- a/usr/src/lib/libzfs/common/libzfs_fru.c    Fri Feb 11 13:58:20 2011 -0500
+++ b/usr/src/lib/libzfs/common/libzfs_fru.c    Mon Feb 14 10:35:37 2011 +0000
@@ -117,9 +117,11 @@
        if (hdl->libzfs_chassis_id[0] == '\0' &&
            strcmp(_topo_node_name(tn), "chassis") == 0) {
                if (_topo_prop_get_string(tn, FM_FMRI_AUTHORITY,
-                   FM_FMRI_AUTH_CHASSIS, &devpath, &err) == 0)
+                   FM_FMRI_AUTH_CHASSIS, &devpath, &err) == 0) {
                        (void) strlcpy(hdl->libzfs_chassis_id, devpath,
                            sizeof (hdl->libzfs_chassis_id));
+                       _topo_hdl_strfree(thp, devpath);
+               }
        }

        /*

Webrev at: http://cr.illumos.org/view/iizapt6d/

Also available in: Atom PDF