[PATCH] iconv: Fix iconv functions not following symlinks [BZ #32339]
Adhemerval Zanella Netto
adhemerval.zanella@linaro.org
Tue Jul 8 17:52:19 GMT 2025
On 25/06/25 01:34, Avinal Kumar wrote:
> I looked into the test case since I am new to glibc, and I can only work on it in my spare time, it will take some time for me. Would you be willing to move this forward? I promise I will put another patch as soon as I am able to create a test case. I can open a Bugzilla issue to track testcase. Thanks.
I think something like the below should suffice as patch regression.
Without the fix the iconvconfig fails and there is no cache file
creation.
diff --git a/iconv/Makefile b/iconv/Makefile
index 9a94a41ba4..fbb7fdb5f5 100644
--- a/iconv/Makefile
+++ b/iconv/Makefile
@@ -85,6 +85,7 @@ tests-special += \
$(objpfx)tst-iconv_prog-buffer-tiny.out \
$(objpfx)tst-iconv_prog-buffer.out \
$(objpfx)tst-iconv_prog.out \
+ $(objpfx)tst-iconvconfig.out \
$(objpfx)tst-translit-mchar.out \
# tests-special
endif
@@ -164,3 +165,8 @@ $(objpfx)tst-iconv_prog-buffer-large.out: \
tst-iconv_prog-buffer.sh $(objpfx)iconv_prog
$(BASH) $< $(common-objdir) '$(run-program-prefix)' '' '22' > $@; \
$(evaluate-test)
+
+$(objpfx)tst-iconvconfig.out: tst-iconvconfig.sh $(objpfx)iconvconfig
+ $(BASH) $< $(common-objdir) '$(test-wrapper-env)' \
+ '$(run-program-env)' > $@; \
+ $(evaluate-test)
diff --git a/iconv/tst-iconvconfig.sh b/iconv/tst-iconvconfig.sh
new file mode 100755
index 0000000000..b242e209cc
--- /dev/null
+++ b/iconv/tst-iconvconfig.sh
@@ -0,0 +1,48 @@
+#!/bin/bash
+# Check if iconvconfig correctly handle config links (BZ 32339)
+# Copyright (C) 2025 Free Software Foundation, Inc.
+# This file is part of the GNU C Library.
+
+# The GNU C Library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+
+# The GNU C Library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# Lesser General Public License for more details.
+
+# You should have received a copy of the GNU Lesser General Public
+# License along with the GNU C Library; if not, see
+# <https://www.gnu.org/licenses/>.
+
+set -e
+
+build_dir=$1
+test_wrapper_env="$2"
+run_program_env="$3"
+
+# We have to have some directories in the library path.
+LIBPATH=$build_dir:$build_dir/iconvdata
+
+ICONVCONFIG="
+$build_dir/elf/ld.so --library-path $LIBPATH $build_dir/iconv/iconvconfig
+"
+
+ICONVCONFIG="$test_wrapper_env $run_program_env $ICONVCONFIG"
+
+TIMEOUTFACTOR=${TIMEOUTFACTOR:-1}
+
+tmpdir=$(mktemp -d $build_dir/iconv/tst-iconvconfig.XXXXXX)
+#trap 'rm -fr $tmpdir' 0 1 2 3 15 EXIT
+
+touch $tmpdir/gconv-modules-extra.conf
+mkdir $tmpdir/gconv-modules.d
+cd $tmpdir/gconv-modules.d && ln -s ../gconv-modules-extra.conf . && cd -
+
+$ICONVCONFIG --nostdlib $tmpdir -o $tmpdir/gconv-modules.cache
+
+[ ! -e $tmpdir/tmpdir/gconv-modules.cache ] || exit 1
+
+exit 0
>
> On Wed, Jun 18, 2025 at 10:11 PM Adhemerval Zanella Netto <adhemerval.zanella@linaro.org <mailto:adhemerval.zanella@linaro.org>> wrote:
>
>
>
> On 16/06/25 06:59, avinal.xlvii@gmail.com <mailto:avinal.xlvii@gmail.com> wrote:
> > From: Avinal Kumar <avinal.xlvii@gmail.com <mailto:avinal.xlvii@gmail.com>>
> >
> > On some file systems iconv do not follow symlinks. This happens because
> > read_conf_file() function's directory traversal loop reject symbolic
> > links and then lstat64() call do not follow symlinks.
> >
> > This commit fixes the directory traversal loop to accept symbolic links
> > and then follow the link using stat64().
> >
> > Signed-off-by: Avinal Kumar <avinal.xlvii@gmail.com <mailto:avinal.xlvii@gmail.com>>
> > ---
> > As per discussion in BZ #32339, these changes should fix the issue. I need help
> > testing it. It would be helpful if John can please test this on the particular
> > system mentioned in the bug report. Looking forward for comments.
>
> LGTM, I was wondering how hard would be to create a testcase. We have
> --nostdlib, so it would a matter to create a temporary directory with a
> configuration and a symlink, and check if the resulting cache has the
> symlink config (which I think it would be most hard part).
>
> > ---
> > iconv/gconv_parseconfdir.h | 8 +++++---
> > 1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/iconv/gconv_parseconfdir.h b/iconv/gconv_parseconfdir.h
> > index a7ebfd80cc..31f8f1c421 100644
> > --- a/iconv/gconv_parseconfdir.h
> > +++ b/iconv/gconv_parseconfdir.h
> > @@ -33,9 +33,10 @@
> > # define closedir __closedir
> > # define mempcpy __mempcpy
> > # define struct_stat64 struct __stat64_t64
> > -# define lstat64 __lstat64_time64
> > +# define stat64_impl __stat64_time64
> > # define feof_unlocked __feof_unlocked
> > #else
> > +# define stat64_impl stat64
> > # define struct_stat64 struct stat64
> > #endif
> >
> > @@ -151,7 +152,8 @@ gconv_parseconfdir (const char *prefix, const char *dir, size_t dir_len)
> > struct dirent64 *ent;
> > while ((ent = readdir64 (confdir)) != NULL)
> > {
> > - if (ent->d_type != DT_REG && ent->d_type != DT_UNKNOWN)
> > + if (ent->d_type != DT_REG && ent->d_type != DT_UNKNOWN
> > + && ent->d_type != DT_LNK)
> > continue;
> >
> > size_t len = strlen (ent->d_name);
> > @@ -166,7 +168,7 @@ gconv_parseconfdir (const char *prefix, const char *dir, size_t dir_len)
> > continue;
> >
> > if (ent->d_type != DT_UNKNOWN
> > - || (lstat64 (conf, &st) != -1 && S_ISREG (st.st_mode)))
> > + || (stat64_impl (conf, &st) != -1 && S_ISREG (st.st_mode)))
> > found |= read_conf_file (conf, dir, dir_len);
> >
> > free (conf);
>
More information about the Libc-alpha
mailing list