diff mbox series

net/mlx5: fix leak in sysfs port name translation

Message ID 20230801151831.3703427-1-david.marchand@redhat.com (mailing list archive)
State Accepted, archived
Delegated to: Raslan Darawsheh
Headers
Series net/mlx5: fix leak in sysfs port name translation |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/intel-Functional success Functional PASS
ci/github-robot: build success github build: passed
ci/iol-aarch-unit-testing success Testing PASS
ci/iol-unit-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS

Commit Message

David Marchand Aug. 1, 2023, 3:18 p.m. UTC 
  getline() may allocate a buffer even though it returns -1:
"""
If  *lineptr  is set to NULL before the call, then getline() will allocate
a buffer for storing the line.  This buffer should be freed by the user
program even if getline() failed.
"""

This leak has been observed on a RHEL8 system with two CX5 PF devices
(no VFs).

ASan reports:
==8899==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 120 byte(s) in 1 object(s) allocated from:
    #0 0x7fe58576aba8 in __interceptor_malloc
	(/lib64/libasan.so.5+0xefba8)
    #1 0x7fe583e866b2 in __getdelim (/lib64/libc.so.6+0x886b2)
    #2 0x327bd23 in mlx5_sysfs_switch_info
	../drivers/net/mlx5/linux/mlx5_ethdev_os.c:1084
    #3 0x3271f86 in mlx5_os_pci_probe_pf
	../drivers/net/mlx5/linux/mlx5_os.c:2282
    #4 0x3273c83 in mlx5_os_pci_probe
	../drivers/net/mlx5/linux/mlx5_os.c:2497
    #5 0x327475f in mlx5_os_net_probe
	../drivers/net/mlx5/linux/mlx5_os.c:2578
    #6 0xc6eac7 in drivers_probe
	../drivers/common/mlx5/mlx5_common.c:937
    #7 0xc6f150 in mlx5_common_dev_probe
	../drivers/common/mlx5/mlx5_common.c:1027
    #8 0xc8ef80 in mlx5_common_pci_probe
	../drivers/common/mlx5/mlx5_common_pci.c:168
    #9 0xc21b67 in rte_pci_probe_one_driver
	../drivers/bus/pci/pci_common.c:312
    #10 0xc2224c in pci_probe_all_drivers
	../drivers/bus/pci/pci_common.c:396
    #11 0xc222f4 in pci_probe ../drivers/bus/pci/pci_common.c:423
    #12 0xb71fff in rte_bus_probe ../lib/eal/common/eal_common_bus.c:78
    #13 0xbe6888 in rte_eal_init ../lib/eal/linux/eal.c:1300
    #14 0x5ec717 in main ../app/test-pmd/testpmd.c:4515
    #15 0x7fe583e38d84 in __libc_start_main (/lib64/libc.so.6+0x3ad84)

As far as why getline() errors, strace gives a hint:
8516  openat(AT_FDCWD, "/sys/class/net/enp130s0f0/phys_port_name",
	O_RDONLY) = 34
8516  fstat(34, {st_mode=S_IFREG|0444, st_size=4096, ...}) = 0
8516  read(34, 0x621000098900, 4096)    = -1 EOPNOTSUPP (Operation
	not supported)

Fixes: f8a226ed65fa ("net/mlx5: fix sysfs port name translation")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/mlx5/linux/mlx5_ethdev_os.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Bing Zhao Aug. 1, 2023, 3:39 p.m. UTC | #1 
Hi David,

Thanks for catching this.

BR. Bing

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Tuesday, August 1, 2023 11:19 PM
> To: dev@dpdk.org
> Cc: stable@dpdk.org; Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
> <viacheslavo@nvidia.com>; Ori Kam <orika@nvidia.com>; Suanming Mou
> <suanmingm@nvidia.com>; Bing Zhao <bingz@nvidia.com>
> Subject: [PATCH] net/mlx5: fix leak in sysfs port name translation
> 
> External email: Use caution opening links or attachments
> 
> 
> getline() may allocate a buffer even though it returns -1:
> """
> If  *lineptr  is set to NULL before the call, then getline() will allocate a buffer for
> storing the line.  This buffer should be freed by the user program even if
> getline() failed.
> """
> 
> This leak has been observed on a RHEL8 system with two CX5 PF devices (no
> VFs).
> 
> ASan reports:
> ==8899==ERROR: LeakSanitizer: detected memory leaks
> 
> Direct leak of 120 byte(s) in 1 object(s) allocated from:
>     #0 0x7fe58576aba8 in __interceptor_malloc
>         (/lib64/libasan.so.5+0xefba8)
>     #1 0x7fe583e866b2 in __getdelim (/lib64/libc.so.6+0x886b2)
>     #2 0x327bd23 in mlx5_sysfs_switch_info
>         ../drivers/net/mlx5/linux/mlx5_ethdev_os.c:1084
>     #3 0x3271f86 in mlx5_os_pci_probe_pf
>         ../drivers/net/mlx5/linux/mlx5_os.c:2282
>     #4 0x3273c83 in mlx5_os_pci_probe
>         ../drivers/net/mlx5/linux/mlx5_os.c:2497
>     #5 0x327475f in mlx5_os_net_probe
>         ../drivers/net/mlx5/linux/mlx5_os.c:2578
>     #6 0xc6eac7 in drivers_probe
>         ../drivers/common/mlx5/mlx5_common.c:937
>     #7 0xc6f150 in mlx5_common_dev_probe
>         ../drivers/common/mlx5/mlx5_common.c:1027
>     #8 0xc8ef80 in mlx5_common_pci_probe
>         ../drivers/common/mlx5/mlx5_common_pci.c:168
>     #9 0xc21b67 in rte_pci_probe_one_driver
>         ../drivers/bus/pci/pci_common.c:312
>     #10 0xc2224c in pci_probe_all_drivers
>         ../drivers/bus/pci/pci_common.c:396
>     #11 0xc222f4 in pci_probe ../drivers/bus/pci/pci_common.c:423
>     #12 0xb71fff in rte_bus_probe ../lib/eal/common/eal_common_bus.c:78
>     #13 0xbe6888 in rte_eal_init ../lib/eal/linux/eal.c:1300
>     #14 0x5ec717 in main ../app/test-pmd/testpmd.c:4515
>     #15 0x7fe583e38d84 in __libc_start_main (/lib64/libc.so.6+0x3ad84)
> 
> As far as why getline() errors, strace gives a hint:
> 8516  openat(AT_FDCWD, "/sys/class/net/enp130s0f0/phys_port_name",
>         O_RDONLY) = 34
> 8516  fstat(34, {st_mode=S_IFREG|0444, st_size=4096, ...}) = 0
> 8516  read(34, 0x621000098900, 4096)    = -1 EOPNOTSUPP (Operation
>         not supported)
> 
> Fixes: f8a226ed65fa ("net/mlx5: fix sysfs port name translation")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  drivers/net/mlx5/linux/mlx5_ethdev_os.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/mlx5/linux/mlx5_ethdev_os.c
> b/drivers/net/mlx5/linux/mlx5_ethdev_os.c
> index 639e629fe4..dd5a0c546d 100644
> --- a/drivers/net/mlx5/linux/mlx5_ethdev_os.c
> +++ b/drivers/net/mlx5/linux/mlx5_ethdev_os.c
> @@ -1083,6 +1083,7 @@ mlx5_sysfs_switch_info(unsigned int ifindex, struct
> mlx5_switch_info *info)
> 
>                 line_size = getline(&port_name, &port_name_size, file);
>                 if (line_size < 0) {
> +                       free(port_name);
>                         fclose(file);
>                         rte_errno = errno;
>                         return -rte_errno;
> --
> 2.41.0
Slava Ovsiienko Aug. 2, 2023, 11:56 a.m. UTC | #2 
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Tuesday, August 1, 2023 6:19 PM
> To: dev@dpdk.org
> Cc: stable@dpdk.org; Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
> <viacheslavo@nvidia.com>; Ori Kam <orika@nvidia.com>; Suanming Mou
> <suanmingm@nvidia.com>; Bing Zhao <bingz@nvidia.com>
> Subject: [PATCH] net/mlx5: fix leak in sysfs port name translation
> 
> getline() may allocate a buffer even though it returns -1:
> """
> If  *lineptr  is set to NULL before the call, then getline() will allocate a buffer
> for storing the line.  This buffer should be freed by the user program even if
> getline() failed.
> """
> 
> This leak has been observed on a RHEL8 system with two CX5 PF devices (no
> VFs).
> 
> ASan reports:
> ==8899==ERROR: LeakSanitizer: detected memory leaks
> 
> Direct leak of 120 byte(s) in 1 object(s) allocated from:
>     #0 0x7fe58576aba8 in __interceptor_malloc
> 	(/lib64/libasan.so.5+0xefba8)
>     #1 0x7fe583e866b2 in __getdelim (/lib64/libc.so.6+0x886b2)
>     #2 0x327bd23 in mlx5_sysfs_switch_info
> 	../drivers/net/mlx5/linux/mlx5_ethdev_os.c:1084
>     #3 0x3271f86 in mlx5_os_pci_probe_pf
> 	../drivers/net/mlx5/linux/mlx5_os.c:2282
>     #4 0x3273c83 in mlx5_os_pci_probe
> 	../drivers/net/mlx5/linux/mlx5_os.c:2497
>     #5 0x327475f in mlx5_os_net_probe
> 	../drivers/net/mlx5/linux/mlx5_os.c:2578
>     #6 0xc6eac7 in drivers_probe
> 	../drivers/common/mlx5/mlx5_common.c:937
>     #7 0xc6f150 in mlx5_common_dev_probe
> 	../drivers/common/mlx5/mlx5_common.c:1027
>     #8 0xc8ef80 in mlx5_common_pci_probe
> 	../drivers/common/mlx5/mlx5_common_pci.c:168
>     #9 0xc21b67 in rte_pci_probe_one_driver
> 	../drivers/bus/pci/pci_common.c:312
>     #10 0xc2224c in pci_probe_all_drivers
> 	../drivers/bus/pci/pci_common.c:396
>     #11 0xc222f4 in pci_probe ../drivers/bus/pci/pci_common.c:423
>     #12 0xb71fff in rte_bus_probe ../lib/eal/common/eal_common_bus.c:78
>     #13 0xbe6888 in rte_eal_init ../lib/eal/linux/eal.c:1300
>     #14 0x5ec717 in main ../app/test-pmd/testpmd.c:4515
>     #15 0x7fe583e38d84 in __libc_start_main (/lib64/libc.so.6+0x3ad84)
> 
> As far as why getline() errors, strace gives a hint:
> 8516  openat(AT_FDCWD, "/sys/class/net/enp130s0f0/phys_port_name",
> 	O_RDONLY) = 34
> 8516  fstat(34, {st_mode=S_IFREG|0444, st_size=4096, ...}) = 0
> 8516  read(34, 0x621000098900, 4096)    = -1 EOPNOTSUPP (Operation
> 	not supported)
> 
> Fixes: f8a226ed65fa ("net/mlx5: fix sysfs port name translation")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  drivers/net/mlx5/linux/mlx5_ethdev_os.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/mlx5/linux/mlx5_ethdev_os.c
> b/drivers/net/mlx5/linux/mlx5_ethdev_os.c
> index 639e629fe4..dd5a0c546d 100644
> --- a/drivers/net/mlx5/linux/mlx5_ethdev_os.c
> +++ b/drivers/net/mlx5/linux/mlx5_ethdev_os.c
> @@ -1083,6 +1083,7 @@ mlx5_sysfs_switch_info(unsigned int ifindex,
> struct mlx5_switch_info *info)
> 
>  		line_size = getline(&port_name, &port_name_size, file);
>  		if (line_size < 0) {
> +			free(port_name);
>  			fclose(file);
>  			rte_errno = errno;
>  			return -rte_errno;
> --
> 2.41.0

Good catch, thank you
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
Raslan Darawsheh Aug. 30, 2023, 12:28 p.m. UTC | #3 
Hi,

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Tuesday, August 1, 2023 6:19 PM
> To: dev@dpdk.org
> Cc: stable@dpdk.org; Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
> <viacheslavo@nvidia.com>; Ori Kam <orika@nvidia.com>; Suanming Mou
> <suanmingm@nvidia.com>; Bing Zhao <bingz@nvidia.com>
> Subject: [PATCH] net/mlx5: fix leak in sysfs port name translation
> 
> getline() may allocate a buffer even though it returns -1:
> """
> If  *lineptr  is set to NULL before the call, then getline() will allocate a buffer for
> storing the line.  This buffer should be freed by the user program even if
> getline() failed.
> """
> 
> This leak has been observed on a RHEL8 system with two CX5 PF devices (no
> VFs).
> 
> ASan reports:
> ==8899==ERROR: LeakSanitizer: detected memory leaks
> 
> Direct leak of 120 byte(s) in 1 object(s) allocated from:
>     #0 0x7fe58576aba8 in __interceptor_malloc
> 	(/lib64/libasan.so.5+0xefba8)
>     #1 0x7fe583e866b2 in __getdelim (/lib64/libc.so.6+0x886b2)
>     #2 0x327bd23 in mlx5_sysfs_switch_info
> 	../drivers/net/mlx5/linux/mlx5_ethdev_os.c:1084
>     #3 0x3271f86 in mlx5_os_pci_probe_pf
> 	../drivers/net/mlx5/linux/mlx5_os.c:2282
>     #4 0x3273c83 in mlx5_os_pci_probe
> 	../drivers/net/mlx5/linux/mlx5_os.c:2497
>     #5 0x327475f in mlx5_os_net_probe
> 	../drivers/net/mlx5/linux/mlx5_os.c:2578
>     #6 0xc6eac7 in drivers_probe
> 	../drivers/common/mlx5/mlx5_common.c:937
>     #7 0xc6f150 in mlx5_common_dev_probe
> 	../drivers/common/mlx5/mlx5_common.c:1027
>     #8 0xc8ef80 in mlx5_common_pci_probe
> 	../drivers/common/mlx5/mlx5_common_pci.c:168
>     #9 0xc21b67 in rte_pci_probe_one_driver
> 	../drivers/bus/pci/pci_common.c:312
>     #10 0xc2224c in pci_probe_all_drivers
> 	../drivers/bus/pci/pci_common.c:396
>     #11 0xc222f4 in pci_probe ../drivers/bus/pci/pci_common.c:423
>     #12 0xb71fff in rte_bus_probe ../lib/eal/common/eal_common_bus.c:78
>     #13 0xbe6888 in rte_eal_init ../lib/eal/linux/eal.c:1300
>     #14 0x5ec717 in main ../app/test-pmd/testpmd.c:4515
>     #15 0x7fe583e38d84 in __libc_start_main (/lib64/libc.so.6+0x3ad84)
> 
> As far as why getline() errors, strace gives a hint:
> 8516  openat(AT_FDCWD, "/sys/class/net/enp130s0f0/phys_port_name",
> 	O_RDONLY) = 34
> 8516  fstat(34, {st_mode=S_IFREG|0444, st_size=4096, ...}) = 0
> 8516  read(34, 0x621000098900, 4096)    = -1 EOPNOTSUPP (Operation
> 	not supported)
> 
> Fixes: f8a226ed65fa ("net/mlx5: fix sysfs port name translation")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh
diff mbox series

Patch

diff --git a/drivers/net/mlx5/linux/mlx5_ethdev_os.c b/drivers/net/mlx5/linux/mlx5_ethdev_os.c
index 639e629fe4..dd5a0c546d 100644
--- a/drivers/net/mlx5/linux/mlx5_ethdev_os.c
+++ b/drivers/net/mlx5/linux/mlx5_ethdev_os.c
@@ -1083,6 +1083,7 @@  mlx5_sysfs_switch_info(unsigned int ifindex, struct mlx5_switch_info *info)
 
 		line_size = getline(&port_name, &port_name_size, file);
 		if (line_size < 0) {
+			free(port_name);
 			fclose(file);
 			rte_errno = errno;
 			return -rte_errno;