[v2] drivers: fix symbol exports when map is omitted
Checks
Commit Message
ld exports any global symbol by default if no version script is passed.
As a consequence, the incriminated change let any public symbol leak
out of the driver shared libraries.
Hide again those symbols by providing a default map file which
unexports any global symbol using a local: * catch-all statement.
The checks are skipped for this default map file as it is intentionnally
an empty map (see commit b67bdda86cd4 ("devtools: catch empty symbol
maps")) and there is nothing else to check in this map.
While at it, move Windows specific objects where needed for better
readability.
Fixes: 7dde9c844a37 ("drivers: omit symbol map when unneeded")
Cc: stable@dpdk.org
Reported-by: Luca Boccassi <luca.boccassi@microsoft.com>
Signed-off-by: David Marchand <david.marchand@redhat.com>
Tested-by: Ferruh Yigit <ferruh.yigit@amd.com>
---
Changes since v1:
- excluded drivers/version.map from maps checked by default in
check-symbol-maps.sh,
---
devtools/check-symbol-maps.sh | 2 +-
drivers/meson.build | 70 +++++++++++++++++++----------------
drivers/version.map | 3 ++
3 files changed, 42 insertions(+), 33 deletions(-)
create mode 100644 drivers/version.map
Comments
On 11/30/2022 10:02 AM, David Marchand wrote:
> ld exports any global symbol by default if no version script is passed.
> As a consequence, the incriminated change let any public symbol leak
> out of the driver shared libraries.
>
> Hide again those symbols by providing a default map file which
> unexports any global symbol using a local: * catch-all statement.
>
> The checks are skipped for this default map file as it is intentionnally
> an empty map (see commit b67bdda86cd4 ("devtools: catch empty symbol
> maps")) and there is nothing else to check in this map.
>
> While at it, move Windows specific objects where needed for better
> readability.
>
> Fixes: 7dde9c844a37 ("drivers: omit symbol map when unneeded")
> Cc: stable@dpdk.org
>
> Reported-by: Luca Boccassi <luca.boccassi@microsoft.com>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> Tested-by: Ferruh Yigit <ferruh.yigit@amd.com>
Tested v2, looks good.
'check-symbol-maps.sh' warning fixed too.
On Wed, Nov 30, 2022 at 11:44 AM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
>
> On 11/30/2022 10:02 AM, David Marchand wrote:
> > ld exports any global symbol by default if no version script is passed.
> > As a consequence, the incriminated change let any public symbol leak
> > out of the driver shared libraries.
> >
> > Hide again those symbols by providing a default map file which
> > unexports any global symbol using a local: * catch-all statement.
> >
> > The checks are skipped for this default map file as it is intentionnally
> > an empty map (see commit b67bdda86cd4 ("devtools: catch empty symbol
> > maps")) and there is nothing else to check in this map.
> >
> > While at it, move Windows specific objects where needed for better
> > readability.
> >
> > Fixes: 7dde9c844a37 ("drivers: omit symbol map when unneeded")
> > Cc: stable@dpdk.org
> >
> > Reported-by: Luca Boccassi <luca.boccassi@microsoft.com>
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > Tested-by: Ferruh Yigit <ferruh.yigit@amd.com>
>
> Tested v2, looks good.
> 'check-symbol-maps.sh' warning fixed too.
Thanks Ferruh.
Bruce / Luca, could you review / confirm it is ok for you?
On Wed, Nov 30, 2022 at 4:02 PM David Marchand
<david.marchand@redhat.com> wrote:
> On Wed, Nov 30, 2022 at 11:44 AM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
> >
> > On 11/30/2022 10:02 AM, David Marchand wrote:
> > > ld exports any global symbol by default if no version script is passed.
> > > As a consequence, the incriminated change let any public symbol leak
> > > out of the driver shared libraries.
> > >
> > > Hide again those symbols by providing a default map file which
> > > unexports any global symbol using a local: * catch-all statement.
> > >
> > > The checks are skipped for this default map file as it is intentionnally
> > > an empty map (see commit b67bdda86cd4 ("devtools: catch empty symbol
> > > maps")) and there is nothing else to check in this map.
> > >
> > > While at it, move Windows specific objects where needed for better
> > > readability.
> > >
> > > Fixes: 7dde9c844a37 ("drivers: omit symbol map when unneeded")
> > > Cc: stable@dpdk.org
> > >
> > > Reported-by: Luca Boccassi <luca.boccassi@microsoft.com>
> > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > Tested-by: Ferruh Yigit <ferruh.yigit@amd.com>
> >
> > Tested v2, looks good.
> > 'check-symbol-maps.sh' warning fixed too.
>
> Thanks Ferruh.
>
> Bruce / Luca, could you review / confirm it is ok for you?
Additionnally, Luca, could you describe how you caught the issue?
Could we enhance the CI to catch this earlier?
On Wed, Nov 30, 2022 at 04:02:26PM +0100, David Marchand wrote:
> On Wed, Nov 30, 2022 at 11:44 AM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
> >
> > On 11/30/2022 10:02 AM, David Marchand wrote:
> > > ld exports any global symbol by default if no version script is passed.
> > > As a consequence, the incriminated change let any public symbol leak
> > > out of the driver shared libraries.
> > >
> > > Hide again those symbols by providing a default map file which
> > > unexports any global symbol using a local: * catch-all statement.
> > >
> > > The checks are skipped for this default map file as it is intentionnally
> > > an empty map (see commit b67bdda86cd4 ("devtools: catch empty symbol
> > > maps")) and there is nothing else to check in this map.
> > >
> > > While at it, move Windows specific objects where needed for better
> > > readability.
> > >
> > > Fixes: 7dde9c844a37 ("drivers: omit symbol map when unneeded")
> > > Cc: stable@dpdk.org
> > >
> > > Reported-by: Luca Boccassi <luca.boccassi@microsoft.com>
> > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > Tested-by: Ferruh Yigit <ferruh.yigit@amd.com>
> >
> > Tested v2, looks good.
> > 'check-symbol-maps.sh' warning fixed too.
>
> Thanks Ferruh.
>
> Bruce / Luca, could you review / confirm it is ok for you?
>
LGTM, thanks.
/Bruce
On Wed, Nov 30, 2022 at 4:42 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Wed, Nov 30, 2022 at 04:02:26PM +0100, David Marchand wrote:
> > On Wed, Nov 30, 2022 at 11:44 AM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
> > >
> > > On 11/30/2022 10:02 AM, David Marchand wrote:
> > > > ld exports any global symbol by default if no version script is passed.
> > > > As a consequence, the incriminated change let any public symbol leak
> > > > out of the driver shared libraries.
> > > >
> > > > Hide again those symbols by providing a default map file which
> > > > unexports any global symbol using a local: * catch-all statement.
> > > >
> > > > The checks are skipped for this default map file as it is intentionnally
> > > > an empty map (see commit b67bdda86cd4 ("devtools: catch empty symbol
> > > > maps")) and there is nothing else to check in this map.
> > > >
> > > > While at it, move Windows specific objects where needed for better
> > > > readability.
> > > >
> > > > Fixes: 7dde9c844a37 ("drivers: omit symbol map when unneeded")
> > > > Cc: stable@dpdk.org
> > > >
> > > > Reported-by: Luca Boccassi <luca.boccassi@microsoft.com>
> > > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > > Tested-by: Ferruh Yigit <ferruh.yigit@amd.com>
> > >
> > > Tested v2, looks good.
> > > 'check-symbol-maps.sh' warning fixed too.
> >
> > Thanks Ferruh.
> >
> > Bruce / Luca, could you review / confirm it is ok for you?
> >
> LGTM, thanks.
Thanks Bruce.
I prefer to separate the "cosmetic" part around Windows (and fix
lib/meson.build too), so I sent a v3 series with only the first patch
marked as backport.
@@ -8,7 +8,7 @@ cd $(dirname $0)/..
export LC_ALL=C
if [ $# = 0 ] ; then
- set -- $(find lib drivers -name '*.map')
+ set -- $(find lib drivers -name '*.map' -a ! -path drivers/version.map)
fi
ret=0
@@ -206,44 +206,50 @@ foreach subpath:subdirs
# now build the shared driver
version_map = '@0@/@1@/version.map'.format(meson.current_source_dir(), drv_path)
- implib = 'lib' + lib_name + '.dll.a'
lk_deps = []
lk_args = []
- if fs.is_file(version_map)
- def_file = custom_target(lib_name + '_def',
- command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
- input: version_map,
- output: '@0@_exports.def'.format(lib_name))
-
- mingw_map = custom_target(lib_name + '_mingw',
- command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
- input: version_map,
- output: '@0@_mingw.map'.format(lib_name))
-
- lk_deps = [version_map, def_file, mingw_map]
- if is_windows
- if is_ms_linker
- lk_args = ['-Wl,/def:' + def_file.full_path()]
- if meson.version().version_compare('<0.54.0')
- lk_args += ['-Wl,/implib:drivers\\' + implib]
- endif
- else
- lk_args = ['-Wl,--version-script=' + mingw_map.full_path()]
+ if not fs.is_file(version_map)
+ version_map = '@0@/version.map'.format(meson.current_source_dir())
+ lk_deps += [version_map]
+ else
+ lk_deps += [version_map]
+ if not is_windows and developer_mode
+ # on unix systems check the output of the
+ # check-symbols.sh script, using it as a
+ # dependency of the .so build
+ lk_deps += custom_target(lib_name + '.sym_chk',
+ command: [check_symbols, version_map, '@INPUT@'],
+ capture: true,
+ input: static_lib,
+ output: lib_name + '.sym_chk')
+ endif
+ endif
+
+ if is_windows
+ if is_ms_linker
+ def_file = custom_target(lib_name + '_def',
+ command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
+ input: version_map,
+ output: '@0@_exports.def'.format(lib_name))
+ lk_deps += def_file
+
+ lk_args = ['-Wl,/def:' + def_file.full_path()]
+ if meson.version().version_compare('<0.54.0')
+ implib = 'lib' + lib_name + '.dll.a'
+ lk_args += ['-Wl,/implib:drivers\\' + implib]
endif
else
- lk_args = ['-Wl,--version-script=' + version_map]
- if developer_mode
- # on unix systems check the output of the
- # check-symbols.sh script, using it as a
- # dependency of the .so build
- lk_deps += custom_target(lib_name + '.sym_chk',
- command: [check_symbols, version_map, '@INPUT@'],
- capture: true,
- input: static_lib,
- output: lib_name + '.sym_chk')
- endif
+ mingw_map = custom_target(lib_name + '_mingw',
+ command: [map_to_win_cmd, '@INPUT@', '@OUTPUT@'],
+ input: version_map,
+ output: '@0@_mingw.map'.format(lib_name))
+ lk_deps += [mingw_map]
+
+ lk_args = ['-Wl,--version-script=' + mingw_map.full_path()]
endif
+ else
+ lk_args = ['-Wl,--version-script=' + version_map]
endif
shared_lib = shared_library(lib_name, sources,
new file mode 100644
@@ -0,0 +1,3 @@
+DPDK_23 {
+ local: *;
+};