| Message ID | 20171126113204.21318-1-aurelien@aurel32.net |
|---|---|
| State | New |
| Headers | show |
| Series | ldconfig: set LC_COLLATE to C | expand |
On 2017-11-28 00:47, Rical Jasan wrote: > On 11/26/2017 03:32 AM, Aurelien Jarno wrote: > > ldconfig supports "include" directives and use the glob function to > > process them. The glob function sort entries according to the LC_COLLATE > > category. When using a standard "include /etc/ld.so.conf.d/*.conf" entry > > in /etc/ld.so.conf, the order therefore depends on the locale used to > > ldconfig. Prefixing the files that have to be processed last with "z" > > or "zz" (as it is often used) therefore doesn't work correctly as "z" > > is not always the last letter. For example in the et_EE locale, it is > > sorted after the "s". > > Did you mean before the "s"? More precisely just after the "s" or between the "s" and the "t". > But wouldn't the user/admin expect the files to be sorted according to > their locale? There might be more than one admin on the system using different locales. In addition to that ldconfig can be launched by system scripts using a different locale. > If a strict ordering is needed, I would expect numerical > prefixing to be used. In practice distributions ship file in /etc/ld.so.conf.d, so it's not something fully in control of users. On Debian and Fedora for example they start with letters. And depending on the locale, numerical prefixes might be sorted before or after letters. Aurelien
On Nov 28 2017, Rical Jasan <ricaljasan@pacific.net> wrote: > But wouldn't the user/admin expect the files to be sorted according to > their locale? If a strict ordering is needed, I would expect numerical > prefixing to be used. I don't think the contents of the (system-wide) ld.so cache should depend on the locale of the admin. Andreas.
This seems like it's fixing an issue that would have been user-visible in releases, so should have a bug filed in Bugzilla.
On 11/26/2017 03:32 AM, Aurelien Jarno wrote: > ldconfig supports "include" directives and use the glob function to > process them. The glob function sort entries according to the LC_COLLATE > category. When using a standard "include /etc/ld.so.conf.d/*.conf" entry > in /etc/ld.so.conf, the order therefore depends on the locale used to > ldconfig. Prefixing the files that have to be processed last with "z" > or "zz" (as it is often used) therefore doesn't work correctly as "z" > is not always the last letter. For example in the et_EE locale, it is > sorted after the "s". > > This patch fixes that by setting LC_COLLATE to C in order to process > files in deterministic order, independently of the locale used to launch > ldconfig. This patch, while being the correct thing to do, has the capability to completely break a users system if the administrator used locale-specific ordering to find paths that were needed for say critical identity management plugins for ldap or other subsystems required for login (krb5), of which I know at least one with such changes (but no locale-specific ordering on the configuration files). A quick brainstorm: * Do nothing. A minimal number of users have broken systems because of collation in their locale breaks certain packages adding known to sort incorrectly entries from their distribution for ld.so.conf.d/. * Switch to C collation and have an unknown number of users with broken systems because they used locale specific sorting for files they added to the ld.so.conf.d/ directory to configure their systems. We should never have been supporting non-C/POSIX collation in this case, because collations for languages changes over time. The Postgresql upstream project is learning this because as glibc locales change their index files are invalidated because they use strcoll. We really should switch to C.UTF-8, but we don't have this upstream yet (working on it). Therefore the safest fix is to switch to C/POSIX collation for now (or forever). We *could* add a flag to ldconfig to return the old behaviour, and distros deploying 2.27 could have the option to enable that via a wrapper script, but I doubt it would be used. It might have value for some kind of continuous deployment where you want to work around such ld.so.conf.d/ scriplets that don't sort properly, but you might as well just fix those instead of work around them. Users with broken systems will only see the breakage if they install such a new glibc, and then run ldconfig, and that is a big change for a stable system. Therefore I think this will be an OK change, but it *must not* be backported to stable branches since it changes expected behaviour. > --- > elf/ldconfig.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/elf/ldconfig.c b/elf/ldconfig.c > index 89042351f8..2c01ab567b 100644 > --- a/elf/ldconfig.c > +++ b/elf/ldconfig.c > @@ -1259,6 +1259,10 @@ main (int argc, char **argv) > /* Set locale via LC_ALL. */ > setlocale (LC_ALL, ""); > > + /* But keep the C collation. That way `include' directives using > + globbing patterns are processed in a locale-independent order. */ > + setlocale (LC_COLLATE, "C"); > + > /* Set the text message domain. */ > textdomain (_libc_intl_domainname); * Needs a bug. This is is a user visible feature (Joseph just commented this too). * Needs a NEWS entry. * Needs a changelog. * Needs a packaging change doc entry: https://sourceware.org/glibc/wiki/Release/2.27#Packaging_Changes OK with those changes. Reviewed-by: Carlos O'Donell <carlos@redhat.com>
On 11/27/2017 05:36 PM, Carlos O'Donell wrote: > A quick brainstorm: > > * Do nothing. A minimal number of users have broken systems because of collation > in their locale breaks certain packages adding known to sort incorrectly entries > from their distribution for ld.so.conf.d/. > > * Switch to C collation and have an unknown number of users with broken systems > because they used locale specific sorting for files they added to the > ld.so.conf.d/ directory to configure their systems. We can also enumerate twice and warn if there are any differences. Or provide a tool like (glibc-system-check?) which reports this and could be extended over time to also check for use of deprecated features/APIs and so on. Thanks, Florian
On 11/27/2017 09:02 AM, Florian Weimer wrote: > On 11/27/2017 05:36 PM, Carlos O'Donell wrote: >> A quick brainstorm: >> >> * Do nothing. A minimal number of users have broken systems because of collation >> in their locale breaks certain packages adding known to sort incorrectly entries >> from their distribution for ld.so.conf.d/. >> >> * Switch to C collation and have an unknown number of users with broken systems >> because they used locale specific sorting for files they added to the >> ld.so.conf.d/ directory to configure their systems. > > We can also enumerate twice and warn if there are any differences. Are you suggesting a 2 release staged change? * First release compare both collations, warn on differences. * Change the release default, and stop warning. It would not make sense to keep warning after the change. > Or provide a tool like (glibc-system-check?) which reports this and > could be extended over time to also check for use of deprecated > features/APIs and so on. Would we use the tool to block upgrades? Detect /etc/ld.so.conf.d/* file names that would sort differently given the current locale and C locale, and return non-zero with a warning?
On 11/27/2017 06:15 PM, Carlos O'Donell wrote: > On 11/27/2017 09:02 AM, Florian Weimer wrote: >> On 11/27/2017 05:36 PM, Carlos O'Donell wrote: >>> A quick brainstorm: >>> >>> * Do nothing. A minimal number of users have broken systems because of collation >>> in their locale breaks certain packages adding known to sort incorrectly entries >>> from their distribution for ld.so.conf.d/. >>> >>> * Switch to C collation and have an unknown number of users with broken systems >>> because they used locale specific sorting for files they added to the >>> ld.so.conf.d/ directory to configure their systems. >> >> We can also enumerate twice and warn if there are any differences. > > Are you suggesting a 2 release staged change? > > * First release compare both collations, warn on differences. > > * Change the release default, and stop warning. > > It would not make sense to keep warning after the change. Why do you say that? System administrators might see the warning and realize immediately that this is an important change. >> Or provide a tool like (glibc-system-check?) which reports this and >> could be extended over time to also check for use of deprecated >> features/APIs and so on. > > Would we use the tool to block upgrades? > > Detect /etc/ld.so.conf.d/* file names that would sort differently > given the current locale and C locale, and return non-zero with > a warning? No, I think it would have to be purely advisory. Thanks, Florian
On 11/27/2017 09:16 AM, Florian Weimer wrote: > On 11/27/2017 06:15 PM, Carlos O'Donell wrote: >> On 11/27/2017 09:02 AM, Florian Weimer wrote: >>> On 11/27/2017 05:36 PM, Carlos O'Donell wrote: >>>> A quick brainstorm: >>>> >>>> * Do nothing. A minimal number of users have broken systems because of collation >>>> in their locale breaks certain packages adding known to sort incorrectly entries >>>> from their distribution for ld.so.conf.d/. >>>> >>>> * Switch to C collation and have an unknown number of users with broken systems >>>> because they used locale specific sorting for files they added to the >>>> ld.so.conf.d/ directory to configure their systems. >>> >>> We can also enumerate twice and warn if there are any differences. >> >> Are you suggesting a 2 release staged change? >> >> * First release compare both collations, warn on differences. >> >> * Change the release default, and stop warning. >> >> It would not make sense to keep warning after the change. > > Why do you say that? > > System administrators might see the warning and realize immediately that this is an important change. Once the change is made to use C/POSIX locale for collation, the warning becomes a false positive. Why issue a warning at that point? No change is required by the system administrator. The warning is important leading up to the change to notify the system administrator that the system behaviour is about to change in the future (particularly if they are on a 2.27-based release and will pickup this change soon e.g. Fedora Rawhide). >>> Or provide a tool like (glibc-system-check?) which reports this and >>> could be extended over time to also check for use of deprecated >>> features/APIs and so on. >> >> Would we use the tool to block upgrades? >> >> Detect /etc/ld.so.conf.d/* file names that would sort differently >> given the current locale and C locale, and return non-zero with >> a warning? > > No, I think it would have to be purely advisory. OK. Given my comments above, do you recommend a 2 release change process? * Deprecate with warnings. * Change default, and remove warning.
On 2017-11-27 08:36, Carlos O'Donell wrote: > On 11/26/2017 03:32 AM, Aurelien Jarno wrote: > > ldconfig supports "include" directives and use the glob function to > > process them. The glob function sort entries according to the LC_COLLATE > > category. When using a standard "include /etc/ld.so.conf.d/*.conf" entry > > in /etc/ld.so.conf, the order therefore depends on the locale used to > > ldconfig. Prefixing the files that have to be processed last with "z" > > or "zz" (as it is often used) therefore doesn't work correctly as "z" > > is not always the last letter. For example in the et_EE locale, it is > > sorted after the "s". > > > > This patch fixes that by setting LC_COLLATE to C in order to process > > files in deterministic order, independently of the locale used to launch > > ldconfig. > > This patch, while being the correct thing to do, has the capability to > completely break a users system if the administrator used locale-specific > ordering to find paths that were needed for say critical identity management > plugins for ldap or other subsystems required for login (krb5), of which > I know at least one with such changes (but no locale-specific ordering on > the configuration files). > > A quick brainstorm: > > * Do nothing. A minimal number of users have broken systems because of collation > in their locale breaks certain packages adding known to sort incorrectly entries > from their distribution for ld.so.conf.d/. Note that even doing nothing, we might break (or unbreak) systems by fixing collation in locales. There have been a lot of good work in that area recently. > * Switch to C collation and have an unknown number of users with broken systems > because they used locale specific sorting for files they added to the > ld.so.conf.d/ directory to configure their systems. > > We should never have been supporting non-C/POSIX collation in this case, because > collations for languages changes over time. The Postgresql upstream project is > learning this because as glibc locales change their index files are invalidated > because they use strcoll. We really should switch to C.UTF-8, but we don't have > this upstream yet (working on it). Even if C.UTF-8 is not supported upstream yet, a few distributions (at least Fedora, Debian and Ubuntu) support it. If we want to support unicode characters, but still having a fixed order, it might be a good idea to add a comment to the code 1) to not forget to do the change at some point 2) to tell such distribution they might already use C.UTF-8 there. > Therefore the safest fix is to switch to C/POSIX collation for now (or forever). > > We *could* add a flag to ldconfig to return the old behaviour, and distros deploying > 2.27 could have the option to enable that via a wrapper script, but I doubt it > would be used. It might have value for some kind of continuous deployment where you > want to work around such ld.so.conf.d/ scriplets that don't sort properly, but > you might as well just fix those instead of work around them. > > Users with broken systems will only see the breakage if they install such a new > glibc, and then run ldconfig, and that is a big change for a stable system. Therefore > I think this will be an OK change, but it *must not* be backported to stable branches > since it changes expected behaviour. > > > --- > > elf/ldconfig.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/elf/ldconfig.c b/elf/ldconfig.c > > index 89042351f8..2c01ab567b 100644 > > --- a/elf/ldconfig.c > > +++ b/elf/ldconfig.c > > @@ -1259,6 +1259,10 @@ main (int argc, char **argv) > > /* Set locale via LC_ALL. */ > > setlocale (LC_ALL, ""); > > > > + /* But keep the C collation. That way `include' directives using > > + globbing patterns are processed in a locale-independent order. */ > > + setlocale (LC_COLLATE, "C"); > > + > > /* Set the text message domain. */ > > textdomain (_libc_intl_domainname); > > * Needs a bug. This is is a user visible feature (Joseph just commented this too). Indeed, I'll do that now. > * Needs a NEWS entry. > > * Needs a changelog. > > * Needs a packaging change doc entry: > https://sourceware.org/glibc/wiki/Release/2.27#Packaging_Changes I'll do that in the next version once we have converged on how to flip the switch. > OK with those changes. > > Reviewed-by: Carlos O'Donell <carlos@redhat.com> Thanks for the review. Aurelien
On 11/26/2017 03:32 AM, Aurelien Jarno wrote: > ldconfig supports "include" directives and use the glob function to > process them. The glob function sort entries according to the LC_COLLATE > category. When using a standard "include /etc/ld.so.conf.d/*.conf" entry > in /etc/ld.so.conf, the order therefore depends on the locale used to > ldconfig. Prefixing the files that have to be processed last with "z" > or "zz" (as it is often used) therefore doesn't work correctly as "z" > is not always the last letter. For example in the et_EE locale, it is > sorted after the "s". Did you mean before the "s"? But wouldn't the user/admin expect the files to be sorted according to their locale? If a strict ordering is needed, I would expect numerical prefixing to be used. Rical
diff --git a/elf/ldconfig.c b/elf/ldconfig.c index 89042351f8..2c01ab567b 100644 --- a/elf/ldconfig.c +++ b/elf/ldconfig.c @@ -1259,6 +1259,10 @@ main (int argc, char **argv) /* Set locale via LC_ALL. */ setlocale (LC_ALL, ""); + /* But keep the C collation. That way `include' directives using + globbing patterns are processed in a locale-independent order. */ + setlocale (LC_COLLATE, "C"); + /* Set the text message domain. */ textdomain (_libc_intl_domainname);