2019-04-02 09:16:57

by Yongcheng Yang

[permalink] [raw]
Subject: [PATCH] nfs-utils: nfsidmap fail to build if no --with-pluginpath specified

The var "pkgplugindir" will be empty if no pluginpath provided.
Makefile "-rpath $(pkgplugindir)" will then get failed.

Signed-off-by: Yongcheng Yang <[email protected]>
---
support/nfsidmap/Makefile.am | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/support/nfsidmap/Makefile.am b/support/nfsidmap/Makefile.am
index cdbeb4c..9c21fa3 100644
--- a/support/nfsidmap/Makefile.am
+++ b/support/nfsidmap/Makefile.am
@@ -1,4 +1,8 @@
+if PATH_PLUGINS
pkgplugindir=$(PATH_PLUGINS)
+else
+pkgplugindir=$(libdir)/libnfsidmap
+endif

if ENABLE_LDAP
UMICH_LDAP_LIB = umich_ldap.la
--
2.17.2



2019-04-02 09:26:09

by Yongcheng Yang

[permalink] [raw]
Subject: Re: [PATCH] nfs-utils: nfsidmap fail to build if no --with-pluginpath specified

Hello Steve,

I'm not sure if this backward fix is okay or there's some better solution.

Here is the testlogs:

###############################
# Previously without the patch:
###############################

[root@ nfs-utils]# git diff
[root@ nfs-utils]# (sh autogen.sh && ./configure ) >/dev/null 2>&1
[root@ nfs-utils]# cd -
/root/bz1624542-use-after-free-in-statd-monitor-list-if-insertion-fails/nfs-utils/support/nfsidmap
[root@ nfsidmap]# make clean && make
test -z "libnfsidmap.la" || rm -f libnfsidmap.la
rm -f ./so_locations
rm -rf .libs _libs
test -z "nsswitch.la static.la " || rm -f nsswitch.la static.la
rm -f ./so_locations
rm -f *.o
rm -f *.lo
...
<snip>
...
/bin/sh ../../libtool --tag=CC --mode=link gcc -pipe -Wall -Wextra -Werror=strict-prototypes -Werror=missing-prototypes -Werror=missing-declarations -Werror=format=2 -Werror=undef -Werror=missing-include-dirs -Werror=strict-aliasing=2 -Werror=init-self -Werror=implicit-function-declaration -Werror=return-type -Werror=switch -Werror=overflow -Werror=parentheses -Werror=aggregate-return -Werror=unused-result -fno-strict-aliasing -g -O2 -module -avoid-version -o nsswitch.la -rpath nss.lo nfsidmap_common.lo ../../support/nfs/libnfsconf.la -lresolv
libtool: link: only absolute run-paths are allowed
make: *** [nsswitch.la] Error 1
[root@ nfsidmap]#
[root@ nfsidmap]#
[root@ nfsidmap]# grep rpath Makefile | grep "pkgplugindir"
Makefile:#am_gums_la_rpath = -rpath $(pkgplugindir)
Makefile:#am_umich_ldap_la_rpath = -rpath $(pkgplugindir)
Makefile: $(AM_V_CCLD)$(nsswitch_la_LINK) -rpath $(pkgplugindir) $(nsswitch_la_OBJECTS) $(nsswitch_la_LIBADD) $(LIBS)
Makefile: $(AM_V_CCLD)$(static_la_LINK) -rpath $(pkgplugindir) $(static_la_OBJECTS) $(static_la_LIBADD) $(LIBS)
[root@ nfsidmap]#


##################
# After this patch:
##################
[root@ nfsidmap]# git diff
diff --git a/support/nfsidmap/Makefile.am b/support/nfsidmap/Makefile.am
index cdbeb4c..9c21fa3 100644
--- a/support/nfsidmap/Makefile.am
+++ b/support/nfsidmap/Makefile.am
@@ -1,4 +1,8 @@
+if PATH_PLUGINS
pkgplugindir=$(PATH_PLUGINS)
+else
+pkgplugindir=$(libdir)/libnfsidmap
+endif

if ENABLE_LDAP
UMICH_LDAP_LIB = umich_ldap.la
[root@ nfsidmap]#
[root@ nfs-utils]# (sh autogen.sh && ./configure ) >/dev/null 2>&1
[root@ nfs-utils]# cd -
/root/bz1624542-use-after-free-in-statd-monitor-list-if-insertion-fails/nfs-utils/support/nfsidmap
[root@ nfsidmap]# make clean && make
test -z "libnfsidmap.la" || rm -f libnfsidmap.la
rm -f ./so_locations
rm -rf .libs _libs
test -z "nsswitch.la static.la " || rm -f nsswitch.la static.la
rm -f ./so_locations
rm -f *.o
rm -f *.lo
...
<snip>
...
/bin/sh ../../libtool --tag=CC --mode=link gcc -pipe -Wall -Wextra -Werror=strict-prototypes -Werror=missing-prototypes -Werror=missing-declarations -Werror=format=2 -Werror=undef -Werror=missing-include-dirs -Werror=strict-aliasing=2 -Werror=init-self -Werror=implicit-function-declaration -Werror=return-type -Werror=switch -Werror=overflow -Werror=parentheses -Werror=aggregate-return -Werror=unused-result -fno-strict-aliasing -g -O2 -module -avoid-version -o nsswitch.la -rpath /usr/lib/libnfsidmap nss.lo nfsidmap_common.lo ../../support/nfs/libnfsconf.la -lresolv
libtool: link: gcc -shared -fPIC -DPIC .libs/nss.o .libs/nfsidmap_common.o -Wl,--whole-archive ../../support/nfs/.libs/libnfsconf.a -Wl,--no-whole-archive -lresolv -O2 -Wl,-soname -Wl,nsswitch.so -o .libs/nsswitch.so
libtool: link: (cd .libs/nsswitch.lax/libnfsconf.a && ar x "/root/bz1624542-use-after-free-in-statd-monitor-list-if-insertion-fails/nfs-utils/support/nfsidmap/../../support/nfs/.libs/libnfsconf.a")
libtool: link: ar cru .libs/nsswitch.a nss.o nfsidmap_common.o .libs/nsswitch.lax/libnfsconf.a/conffile.o .libs/nsswitch.lax/libnfsconf.a/xlog.o
libtool: link: ranlib .libs/nsswitch.a
libtool: link: rm -fr .libs/nsswitch.lax
libtool: link: ( cd ".libs" && rm -f "nsswitch.la" && ln -s "../nsswitch.la" "nsswitch.la" )
/bin/sh ../../libtool --tag=CC --mode=compile gcc -DHAVE_CONFIG_H -I. -I../../support/include -I/usr/include/tirpc -D_GNU_SOURCE -pipe -Wall -Wextra -Werror=strict-prototypes -Werror=missing-prototypes -Werror=missing-declarations -Werror=format=2 -Werror=undef -Werror=missing-include-dirs -Werror=strict-aliasing=2 -Werror=init-self -Werror=implicit-function-declaration -Werror=return-type -Werror=switch -Werror=overflow -Werror=parentheses -Werror=aggregate-return -Werror=unused-result -fno-strict-aliasing -g -O2 -MT static.lo -MD -MP -MF .deps/static.Tpo -c -o static.lo static.c
libtool: compile: gcc -DHAVE_CONFIG_H -I. -I../../support/include -I/usr/include/tirpc -D_GNU_SOURCE -pipe -Wall -Wextra -Werror=strict-prototypes -Werror=missing-prototypes -Werror=missing-declarations -Werror=format=2 -Werror=undef -Werror=missing-include-dirs -Werror=strict-aliasing=2 -Werror=init-self -Werror=implicit-function-declaration -Werror=return-type -Werror=switch -Werror=overflow -Werror=parentheses -Werror=aggregate-return -Werror=unused-result -fno-strict-aliasing -g -O2 -MT static.lo -MD -MP -MF .deps/static.Tpo -c static.c -fPIC -DPIC -o .libs/static.o
libtool: compile: gcc -DHAVE_CONFIG_H -I. -I../../support/include -I/usr/include/tirpc -D_GNU_SOURCE -pipe -Wall -Wextra -Werror=strict-prototypes -Werror=missing-prototypes -Werror=missing-declarations -Werror=format=2 -Werror=undef -Werror=missing-include-dirs -Werror=strict-aliasing=2 -Werror=init-self -Werror=implicit-function-declaration -Werror=return-type -Werror=switch -Werror=overflow -Werror=parentheses -Werror=aggregate-return -Werror=unused-result -fno-strict-aliasing -g -O2 -MT static.lo -MD -MP -MF .deps/static.Tpo -c static.c -o static.o >/dev/null 2>&1
mv -f .deps/static.Tpo .deps/static.Plo
/bin/sh ../../libtool --tag=CC --mode=link gcc -pipe -Wall -Wextra -Werror=strict-prototypes -Werror=missing-prototypes -Werror=missing-declarations -Werror=format=2 -Werror=undef -Werror=missing-include-dirs -Werror=strict-aliasing=2 -Werror=init-self -Werror=implicit-function-declaration -Werror=return-type -Werror=switch -Werror=overflow -Werror=parentheses -Werror=aggregate-return -Werror=unused-result -fno-strict-aliasing -g -O2 -module -avoid-version -o static.la -rpath /usr/lib/libnfsidmap static.lo ../../support/nfs/libnfsconf.la -lresolv
libtool: link: gcc -shared -fPIC -DPIC .libs/static.o -Wl,--whole-archive ../../support/nfs/.libs/libnfsconf.a -Wl,--no-whole-archive -lresolv -O2 -Wl,-soname -Wl,static.so -o .libs/static.so
libtool: link: (cd .libs/static.lax/libnfsconf.a && ar x "/root/bz1624542-use-after-free-in-statd-monitor-list-if-insertion-fails/nfs-utils/support/nfsidmap/../../support/nfs/.libs/libnfsconf.a")
libtool: link: ar cru .libs/static.a static.o .libs/static.lax/libnfsconf.a/conffile.o .libs/static.lax/libnfsconf.a/xlog.o
libtool: link: ranlib .libs/static.a
libtool: link: rm -fr .libs/static.lax
libtool: link: ( cd ".libs" && rm -f "static.la" && ln -s "../static.la" "static.la" )
[root@ nfsidmap]# echo $?
0

2019-04-16 14:34:53

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH] nfs-utils: nfsidmap fail to build if no --with-pluginpath specified

Hello,

On 4/2/19 5:16 AM, Yongcheng Yang wrote:
> The var "pkgplugindir" will be empty if no pluginpath provided.
> Makefile "-rpath $(pkgplugindir)" will then get failed.
>
> Signed-off-by: Yongcheng Yang <[email protected]>
> ---
> support/nfsidmap/Makefile.am | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/support/nfsidmap/Makefile.am b/support/nfsidmap/Makefile.am
> index cdbeb4c..9c21fa3 100644
> --- a/support/nfsidmap/Makefile.am
> +++ b/support/nfsidmap/Makefile.am
> @@ -1,4 +1,8 @@
> +if PATH_PLUGINS
> pkgplugindir=$(PATH_PLUGINS)
> +else
> +pkgplugindir=$(libdir)/libnfsidmap
> +endif
>
> if ENABLE_LDAP
> UMICH_LDAP_LIB = umich_ldap.la
>
The only way I see a failure is when ./configure --with-pluginpath=
without a path which is wrong...

./configure --with-pluginpath=/usr/lib/libnfsidmap seems to work
just fine

I thinking it is better to error out when a path is not given
then to gloss over but used a default.

steved.


2019-04-17 09:22:52

by Yongcheng Yang

[permalink] [raw]
Subject: Re: [PATCH] nfs-utils: nfsidmap fail to build if no --with-pluginpath specified

Hi SteveD,

On Tue, Apr 16, 2019 at 10:34:51AM -0400, Steve Dickson wrote:
> The only way I see a failure is when ./configure --with-pluginpath=
> without a path which is wrong...
>
> ./configure --with-pluginpath=/usr/lib/libnfsidmap seems to work
> just fine
>
> I thinking it is better to error out when a path is not given
> then to gloss over but used a default.
>
> steved.
>

From what you comments, there should be a plugin path assigned by
default if the user is not aware of it, correct?

Surely "./configure --with-pluginpath=/usr/lib/libnfsidmap" can work.
But I didn't know the specific exact path before.

I was just using "./configure" without any options and find out that
it's empty. (Please find my previous full logs)

Agree with you we should "use a default" one when path is not given.

Thanks,
Yongcheng

2019-05-10 18:50:33

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH] nfs-utils: nfsidmap fail to build if no --with-pluginpath specified



On 4/17/19 5:22 AM, Yongcheng Yang wrote:
> Hi SteveD,
>
> On Tue, Apr 16, 2019 at 10:34:51AM -0400, Steve Dickson wrote:
>> The only way I see a failure is when ./configure --with-pluginpath=
>> without a path which is wrong...
>>
>> ./configure --with-pluginpath=/usr/lib/libnfsidmap seems to work
>> just fine
>>
>> I thinking it is better to error out when a path is not given
>> then to gloss over but used a default.
>>
>> steved.
>>
>
> From what you comments, there should be a plugin path assigned by
> default if the user is not aware of it, correct?
>
> Surely "./configure --with-pluginpath=/usr/lib/libnfsidmap" can work.
> But I didn't know the specific exact path before.
>
> I was just using "./configure" without any options and find out that
> it's empty. (Please find my previous full logs)
>
> Agree with you we should "use a default" one when path is not given.
Change of mind.... I think the patch is good...

Committed! thanks!!

steved.
>
> Thanks,
> Yongcheng
>