ibm-power-utilities / powerpc-utils

Suite of utilities for Linux on Power systems
GNU General Public License v2.0
34 stars 55 forks source link

Valgrind reports memory leak in lsslot when listing pci and vio slots #78

Closed tyreld closed 2 years ago

tyreld commented 2 years ago
==2102767== Memcheck, a memory error detector
==2102767== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==2102767== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==2102767== Command: src/drmgr/lsslot -c slot
==2102767== 
# Slot                      Description       Linux Name    Device(s)
U9040.MR9.132944X-V16-C0    Virtual I/O Slot  30000000      vty
U9040.MR9.132944X-V16-C11   Virtual I/O Slot  3000000b      l-lan
U9040.MR9.132944X-V16-C101  Virtual I/O Slot  30000065      v-scsi
U9040.MR9.132944X-V16-C312  Virtual I/O Slot  30000138      vfc-client
U9040.MR9.132944X-V16-C313  Virtual I/O Slot  30000139      vfc-client
==2102767== 
==2102767== HEAP SUMMARY:
==2102767==     in use at exit: 64 bytes in 2 blocks
==2102767==   total heap usage: 5,203 allocs, 5,201 frees, 203,943,832 bytes allocated
==2102767== 
==2102767== 26 bytes in 1 blocks are definitely lost in loss record 1 of 2
==2102767==    at 0x40A53B8: malloc (vg_replace_malloc.c:381)
==2102767==    by 0x429F2F3: strdup (in /usr/lib64/libc.so.6)
==2102767==    by 0x1000F3A3: of_to_full_path (common_ofdt.c:456)
==2102767==    by 0x1000F5CF: get_drc_info (common_ofdt.c:499)
==2102767==    by 0x1000BEBF: add_pci_vio_node (common_pci.c:632)
==2102767==    by 0x1000D307: get_dlpar_nodes (common_pci.c:856)
==2102767==    by 0x10003083: lsslot_chrp_pci (lsslot.c:568)
==2102767==    by 0x100047DF: main (lsslot.c:965)
==2102767== 
==2102767== 38 bytes in 1 blocks are definitely lost in loss record 2 of 2
==2102767==    at 0x40A53B8: malloc (vg_replace_malloc.c:381)
==2102767==    by 0x429F2F3: strdup (in /usr/lib64/libc.so.6)
==2102767==    by 0x1000F3A3: of_to_full_path (common_ofdt.c:456)
==2102767==    by 0x1000F5CF: get_drc_info (common_ofdt.c:499)
==2102767==    by 0x1000BEBF: add_pci_vio_node (common_pci.c:632)
==2102767==    by 0x1000D337: get_dlpar_nodes (common_pci.c:859)
==2102767==    by 0x10003083: lsslot_chrp_pci (lsslot.c:568)
==2102767==    by 0x100047DF: main (lsslot.c:965)
==2102767== 
==2102767== LEAK SUMMARY:
==2102767==    definitely lost: 64 bytes in 2 blocks
==2102767==    indirectly lost: 0 bytes in 0 blocks
==2102767==      possibly lost: 0 bytes in 0 blocks
==2102767==    still reachable: 0 bytes in 0 blocks
==2102767==         suppressed: 0 bytes in 0 blocks
==2102767== 
==2102767== For lists of detected and suppressed errors, rerun with: -s
==2102767== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)
tyreld commented 2 years ago
get_drc_info(const char *of_path)                                                
{                                                                                
        struct stat sbuf;                                                        
        char fname[DR_PATH_MAX];                                                 
        char ofdt_path[DR_PATH_MAX];                                             
        char *full_path = NULL;                                                  
        struct dr_connector *list = NULL;                                        
        int rc;                                                                  

        for (list = all_drc_lists; list; list = list->all_next) {                
                if (! strcmp(list->ofdt_path, of_path))                          
                        return list;                                             
        }                                                                        

        full_path = of_to_full_path(of_path);                                    
        if (full_path == NULL)                                                   
                return NULL;                                                     

        /* ibm,drc-info vs the old implementation */                             
        sprintf(fname, "%s/%s", full_path, "ibm,drc-info");                      
        snprintf(ofdt_path, DR_PATH_MAX, "%s", of_path);                         
        rc = stat(fname, &sbuf);                                                 
        if (rc)                                                                  
                rc = drc_info_connectors_v1(full_path, ofdt_path, &list);        
        else                                                                     
                rc = drc_info_connectors_v2(full_path, ofdt_path, &list);        

        if (rc == 0) {                                                           
                list->all_next = all_drc_lists;                                  
                all_drc_lists = list;                                            
        } else {                                                                 
                if (full_path)                                                   
                        free(full_path);                                         
                list = NULL;                                                     
        }                                                                        

        return list;                                                             
}

full_path is only freed if drc_info_connectors_vX() fails. It should be freed regardless. Further, the if (full_path) check is unnecessary as we return immediately if full_path is NULL following of_to_full_path() call.

tyreld commented 2 years ago

This is in src/drmgr common code so I can only assume the same leaks are observed running drmgr.

tyreld commented 2 years ago

Fixed by following commit:

313bb6fdc145 lsslot: fix memory leak when listing IO slots