* [dpdk-dev] [PATCH] eal: Copy raw strings taken from command line
@ 2017-08-04 18:53 Patrick MacArthur
2017-09-04 10:12 ` Sergio Gonzalez Monroy
0 siblings, 1 reply; 3+ messages in thread
From: Patrick MacArthur @ 2017-08-04 18:53 UTC (permalink / raw)
To: dev; +Cc: stable
Normally, command line argument strings are considered immutable, but
SPDK [1] and urdma [2] construct argv arrays to pass to rte_eal_init().
These strings are allocated using malloc() and freed after DPDK
initialization with free(). However, in the case of --file-prefix and
--huge-dir, DPDK takes the pointer to these strings in argv directly. If
a secondary process calls rte_eal_pci_probe() after rte_eal_init()
returns, as is done by SPDK, this causes a use-after-free error because
the strings have been freed by the calling code immediately after
rte_eal_init() returns.
This problem was observed when running SPDK example programs as a
secondary process and causes the secondary processes to fail:
> Starting DPDK 16.11.1 initialization...
> [ DPDK EAL parameters: identify -c 4 --file-prefix=spdk3260 --base-virtaddr=0x1000000000 --proc-type=auto ]
> EAL: Detected 40 lcore(s)
> EAL: Auto-detected process type: SECONDARY
> EAL: Probing VFIO support...
> EAL: VFIO support initialized
> EAL: PCI device 0000:81:00.0 on NUMA socket 1
> EAL: probe driver: 8086:953 spdk_nvme
> EAL: cannot connect to primary process!
> EAL: Error - exiting with code: 1
> Cause: Requested device 0000:81:00.0 cannot be used
Running strace shows that the file prefix has been zero'd out by the
time that the secondary process attempts to probe the NVMe device.
The use-after-free errors can be easily detected with valgrind:
> ==8489== Invalid read of size 1
> ==8489== at 0x4C30D22: strlen (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==8489== by 0x58DB955: vfprintf (vfprintf.c:1637)
> ==8489== by 0x59A4685: __vsnprintf_chk (vsnprintf_chk.c:63)
> ==8489== by 0x59A45E7: __snprintf_chk (snprintf_chk.c:34)
> ==8489== by 0x1246AB: get_socket_path.constprop.0 (in /home/pmacarth/src/spdk/examples/nvme/identify/identify)
> ==8489== by 0x124B09: vfio_mp_sync_connect_to_primary (in /home/pmacarth/src/spdk/examples/nvme/identify/identify)
> ==8489== by 0x123BE4: vfio_get_group_fd.part.1 (in /home/pmacarth/src/spdk/examples/nvme/identify/identify)
> ==8489== by 0x124366: vfio_setup_device (in /home/pmacarth/src/spdk/examples/nvme/identify/identify)
> ==8489== by 0x126C8A: pci_vfio_map_resource (in /home/pmacarth/src/spdk/examples/nvme/identify/identify)
> ==8489== by 0x12B115: pci_probe_all_drivers.part.0 (in /home/pmacarth/src/spdk/examples/nvme/identify/identify)
> ==8489== by 0x12B596: rte_eal_pci_probe (in /home/pmacarth/src/spdk/examples/nvme/identify/identify)
> ==8489== by 0x11D5B5: spdk_pci_enumerate (pci.c:147)
> ==8489== Address 0x63f362e is 14 bytes inside a block of size 32 free'd
> ==8489== at 0x4C2ED5B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==8489== by 0x11E6FB: spdk_free_args (init.c:136)
> ==8489== by 0x11EBF5: spdk_env_init (init.c:309)
> ==8489== by 0x10D2AA: main (identify.c:976)
> ==8489== Block was alloc'd at
> ==8489== at 0x4C2DB2F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==8489== by 0x11E7D7: _sprintf_alloc (init.c:76)
> ==8489== by 0x11EA78: spdk_build_eal_cmdline (init.c:251)
> ==8489== by 0x11EA78: spdk_env_init (init.c:282)
> ==8489== by 0x10D2AA: main (identify.c:976)
> ==8489==
Fix this by using strdup() to create separate memory buffers for these
strings. Note that this patch will cause valgrind to report memory
leaks of these buffers as there is nowhere to free them. Using static
buffers is an option but would make these strings have a fixed maximum
length whereas there is currently no limit defined by the API.
[1] http://spdk.io
[2] https://github.com/zrlio/urdma
Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org
Signed-off-by: Patrick MacArthur <patrick@patrickmacarthur.net>
---
lib/librte_eal/linuxapp/eal/eal.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 48f12f44cfa4..529d2cebc84b 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -569,11 +569,11 @@ eal_parse_args(int argc, char **argv)
break;
case OPT_HUGE_DIR_NUM:
- internal_config.hugepage_dir = optarg;
+ internal_config.hugepage_dir = strdup(optarg);
break;
case OPT_FILE_PREFIX_NUM:
- internal_config.hugefile_prefix = optarg;
+ internal_config.hugefile_prefix = strdup(optarg);
break;
case OPT_SOCKET_MEM_NUM:
--
2.11.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [dpdk-dev] [PATCH] eal: Copy raw strings taken from command line
2017-08-04 18:53 [dpdk-dev] [PATCH] eal: Copy raw strings taken from command line Patrick MacArthur
@ 2017-09-04 10:12 ` Sergio Gonzalez Monroy
2017-10-09 21:27 ` Thomas Monjalon
0 siblings, 1 reply; 3+ messages in thread
From: Sergio Gonzalez Monroy @ 2017-09-04 10:12 UTC (permalink / raw)
To: Patrick MacArthur, dev; +Cc: stable
On 04/08/2017 19:53, Patrick MacArthur wrote:
> Normally, command line argument strings are considered immutable, but
> SPDK [1] and urdma [2] construct argv arrays to pass to rte_eal_init().
> These strings are allocated using malloc() and freed after DPDK
> initialization with free(). However, in the case of --file-prefix and
> --huge-dir, DPDK takes the pointer to these strings in argv directly. If
> a secondary process calls rte_eal_pci_probe() after rte_eal_init()
> returns, as is done by SPDK, this causes a use-after-free error because
> the strings have been freed by the calling code immediately after
> rte_eal_init() returns.
>
> This problem was observed when running SPDK example programs as a
> secondary process and causes the secondary processes to fail:
>
>> Starting DPDK 16.11.1 initialization...
>> [ DPDK EAL parameters: identify -c 4 --file-prefix=spdk3260 --base-virtaddr=0x1000000000 --proc-type=auto ]
>> EAL: Detected 40 lcore(s)
>> EAL: Auto-detected process type: SECONDARY
>> EAL: Probing VFIO support...
>> EAL: VFIO support initialized
>> EAL: PCI device 0000:81:00.0 on NUMA socket 1
>> EAL: probe driver: 8086:953 spdk_nvme
>> EAL: cannot connect to primary process!
>> EAL: Error - exiting with code: 1
>> Cause: Requested device 0000:81:00.0 cannot be used
> Running strace shows that the file prefix has been zero'd out by the
> time that the secondary process attempts to probe the NVMe device.
>
> The use-after-free errors can be easily detected with valgrind:
>
>> ==8489== Invalid read of size 1
>> ==8489== at 0x4C30D22: strlen (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
>> ==8489== by 0x58DB955: vfprintf (vfprintf.c:1637)
>> ==8489== by 0x59A4685: __vsnprintf_chk (vsnprintf_chk.c:63)
>> ==8489== by 0x59A45E7: __snprintf_chk (snprintf_chk.c:34)
>> ==8489== by 0x1246AB: get_socket_path.constprop.0 (in /home/pmacarth/src/spdk/examples/nvme/identify/identify)
>> ==8489== by 0x124B09: vfio_mp_sync_connect_to_primary (in /home/pmacarth/src/spdk/examples/nvme/identify/identify)
>> ==8489== by 0x123BE4: vfio_get_group_fd.part.1 (in /home/pmacarth/src/spdk/examples/nvme/identify/identify)
>> ==8489== by 0x124366: vfio_setup_device (in /home/pmacarth/src/spdk/examples/nvme/identify/identify)
>> ==8489== by 0x126C8A: pci_vfio_map_resource (in /home/pmacarth/src/spdk/examples/nvme/identify/identify)
>> ==8489== by 0x12B115: pci_probe_all_drivers.part.0 (in /home/pmacarth/src/spdk/examples/nvme/identify/identify)
>> ==8489== by 0x12B596: rte_eal_pci_probe (in /home/pmacarth/src/spdk/examples/nvme/identify/identify)
>> ==8489== by 0x11D5B5: spdk_pci_enumerate (pci.c:147)
>> ==8489== Address 0x63f362e is 14 bytes inside a block of size 32 free'd
>> ==8489== at 0x4C2ED5B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
>> ==8489== by 0x11E6FB: spdk_free_args (init.c:136)
>> ==8489== by 0x11EBF5: spdk_env_init (init.c:309)
>> ==8489== by 0x10D2AA: main (identify.c:976)
>> ==8489== Block was alloc'd at
>> ==8489== at 0x4C2DB2F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
>> ==8489== by 0x11E7D7: _sprintf_alloc (init.c:76)
>> ==8489== by 0x11EA78: spdk_build_eal_cmdline (init.c:251)
>> ==8489== by 0x11EA78: spdk_env_init (init.c:282)
>> ==8489== by 0x10D2AA: main (identify.c:976)
>> ==8489==
> Fix this by using strdup() to create separate memory buffers for these
> strings. Note that this patch will cause valgrind to report memory
> leaks of these buffers as there is nowhere to free them. Using static
> buffers is an option but would make these strings have a fixed maximum
> length whereas there is currently no limit defined by the API.
>
> [1] http://spdk.io
> [2] https://github.com/zrlio/urdma
>
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
>
> Signed-off-by: Patrick MacArthur <patrick@patrickmacarthur.net>
> ---
> lib/librte_eal/linuxapp/eal/eal.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
> index 48f12f44cfa4..529d2cebc84b 100644
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> @@ -569,11 +569,11 @@ eal_parse_args(int argc, char **argv)
> break;
>
> case OPT_HUGE_DIR_NUM:
> - internal_config.hugepage_dir = optarg;
> + internal_config.hugepage_dir = strdup(optarg);
> break;
>
> case OPT_FILE_PREFIX_NUM:
> - internal_config.hugefile_prefix = optarg;
> + internal_config.hugefile_prefix = strdup(optarg);
> break;
>
> case OPT_SOCKET_MEM_NUM:
Acked-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [dpdk-dev] [PATCH] eal: Copy raw strings taken from command line
2017-09-04 10:12 ` Sergio Gonzalez Monroy
@ 2017-10-09 21:27 ` Thomas Monjalon
0 siblings, 0 replies; 3+ messages in thread
From: Thomas Monjalon @ 2017-10-09 21:27 UTC (permalink / raw)
To: Patrick MacArthur; +Cc: dev, Sergio Gonzalez Monroy, stable
04/09/2017 12:12, Sergio Gonzalez Monroy:
> On 04/08/2017 19:53, Patrick MacArthur wrote:
> > Normally, command line argument strings are considered immutable, but
> > SPDK [1] and urdma [2] construct argv arrays to pass to rte_eal_init().
> > These strings are allocated using malloc() and freed after DPDK
> > initialization with free(). However, in the case of --file-prefix and
> > --huge-dir, DPDK takes the pointer to these strings in argv directly. If
> > a secondary process calls rte_eal_pci_probe() after rte_eal_init()
> > returns, as is done by SPDK, this causes a use-after-free error because
> > the strings have been freed by the calling code immediately after
> > rte_eal_init() returns.
[...]
> > Fix this by using strdup() to create separate memory buffers for these
> > strings. Note that this patch will cause valgrind to report memory
> > leaks of these buffers as there is nowhere to free them. Using static
> > buffers is an option but would make these strings have a fixed maximum
> > length whereas there is currently no limit defined by the API.
> >
> > [1] http://spdk.io
> > [2] https://github.com/zrlio/urdma
> >
> > Fixes: af75078fece3 ("first public release")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Patrick MacArthur <patrick@patrickmacarthur.net>
>
> Acked-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
Applied, thanks
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-10-09 21:27 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-04 18:53 [dpdk-dev] [PATCH] eal: Copy raw strings taken from command line Patrick MacArthur
2017-09-04 10:12 ` Sergio Gonzalez Monroy
2017-10-09 21:27 ` Thomas Monjalon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).