* [PATCH] elf: ldconfig should skip temporary files created by package managers
@ 2023-10-20 12:29 Florian Weimer
2023-10-20 12:48 ` Adhemerval Zanella Netto
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Florian Weimer @ 2023-10-20 12:29 UTC (permalink / raw)
To: libc-alpha
This avoids crashes due to partially written files, after a package
update is interrupted.
Tested on x86_64-linux-gnu.
---
NEWS | 4 +++-
elf/ldconfig.c | 39 +++++++++++++++++++++++++++------------
2 files changed, 30 insertions(+), 13 deletions(-)
diff --git a/NEWS b/NEWS
index cc4b81f0ac..9432564444 100644
--- a/NEWS
+++ b/NEWS
@@ -40,7 +40,9 @@ Major new features:
Deprecated and removed features, and other changes affecting compatibility:
- [Add deprecations, removals and changes affecting compatibility here]
+* The ldconfig program now skips file names containing ';' or ending in
+ ".tmp", to avoid examining temporary files created by the RPM and dpkg
+ package managers.
Changes to build and runtime requirements:
diff --git a/elf/ldconfig.c b/elf/ldconfig.c
index d26eef1fb4..02387a169c 100644
--- a/elf/ldconfig.c
+++ b/elf/ldconfig.c
@@ -661,6 +661,31 @@ struct dlib_entry
struct dlib_entry *next;
};
+/* Skip some temporary DSO files. These files may be partially written
+ and lead to ldconfig crashes when examined. */
+static bool
+skip_dso_based_on_name (const char *name, size_t len)
+{
+ /* Skip temporary files created by the prelink program. Files with
+ names like these are never really DSOs we want to look at. */
+ if (len >= sizeof (".#prelink#") - 1)
+ {
+ if (strcmp (name + len - sizeof (".#prelink#") + 1,
+ ".#prelink#") == 0)
+ return true;
+ if (len >= sizeof (".#prelink#.XXXXXX") - 1
+ && memcmp (name + len - sizeof (".#prelink#.XXXXXX")
+ + 1, ".#prelink#.", sizeof (".#prelink#.") - 1) == 0)
+ return true;
+ }
+ /* Skip temporary files created by RPM. */
+ if (memchr (name, len, ';') != NULL)
+ return true;
+ /* Skip temporary files created by dpkg. */
+ if (len > 4 && memcmp (name + len - 4, ".tmp", 4) == 0)
+ return true;
+ return false;
+}
static void
search_dir (const struct dir_entry *entry)
@@ -711,18 +736,8 @@ search_dir (const struct dir_entry *entry)
continue;
size_t len = strlen (direntry->d_name);
- /* Skip temporary files created by the prelink program. Files with
- names like these are never really DSOs we want to look at. */
- if (len >= sizeof (".#prelink#") - 1)
- {
- if (strcmp (direntry->d_name + len - sizeof (".#prelink#") + 1,
- ".#prelink#") == 0)
- continue;
- if (len >= sizeof (".#prelink#.XXXXXX") - 1
- && memcmp (direntry->d_name + len - sizeof (".#prelink#.XXXXXX")
- + 1, ".#prelink#.", sizeof (".#prelink#.") - 1) == 0)
- continue;
- }
+ if (skip_dso_based_on_name (direntry->d_name, len))
+ continue;
if (asprintf (&file_name, "%s/%s", entry->path, direntry->d_name) < 0)
error (EXIT_FAILURE, errno, _("Could not form library path"));
if (opt_chroot != NULL)
base-commit: f5677d9cebb12edcd9301dbb5cf40f82618b46af
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] elf: ldconfig should skip temporary files created by package managers
2023-10-20 12:29 [PATCH] elf: ldconfig should skip temporary files created by package managers Florian Weimer
@ 2023-10-20 12:48 ` Adhemerval Zanella Netto
2023-10-20 14:41 ` Florian Weimer
2023-10-20 21:33 ` Guillem Jover
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Adhemerval Zanella Netto @ 2023-10-20 12:48 UTC (permalink / raw)
To: Florian Weimer, libc-alpha
On 20/10/23 09:29, Florian Weimer wrote:
> This avoids crashes due to partially written files, after a package
> update is interrupted.
It seems kinda unreliable a tool create temporary files on the libdir,
although we alrady handle for prelinkk so it makes sense to extend to
other program as well. Maybe another option would to proper parse the
file header and only handle ELF files, so ldconfig is not bounded to
name patterns.
>
> Tested on x86_64-linux-gnu.
>
> ---
> NEWS | 4 +++-
> elf/ldconfig.c | 39 +++++++++++++++++++++++++++------------
> 2 files changed, 30 insertions(+), 13 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index cc4b81f0ac..9432564444 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -40,7 +40,9 @@ Major new features:
>
> Deprecated and removed features, and other changes affecting compatibility:
>
> - [Add deprecations, removals and changes affecting compatibility here]
> +* The ldconfig program now skips file names containing ';' or ending in
> + ".tmp", to avoid examining temporary files created by the RPM and dpkg
> + package managers.
>
> Changes to build and runtime requirements:
>
> diff --git a/elf/ldconfig.c b/elf/ldconfig.c
> index d26eef1fb4..02387a169c 100644
> --- a/elf/ldconfig.c
> +++ b/elf/ldconfig.c
> @@ -661,6 +661,31 @@ struct dlib_entry
> struct dlib_entry *next;
> };
>
> +/* Skip some temporary DSO files. These files may be partially written
> + and lead to ldconfig crashes when examined. */
> +static bool
> +skip_dso_based_on_name (const char *name, size_t len)
> +{
> + /* Skip temporary files created by the prelink program. Files with
> + names like these are never really DSOs we want to look at. */
> + if (len >= sizeof (".#prelink#") - 1)
> + {
> + if (strcmp (name + len - sizeof (".#prelink#") + 1,
> + ".#prelink#") == 0)
> + return true;
> + if (len >= sizeof (".#prelink#.XXXXXX") - 1
> + && memcmp (name + len - sizeof (".#prelink#.XXXXXX")
> + + 1, ".#prelink#.", sizeof (".#prelink#.") - 1) == 0)
> + return true;
> + }
> + /* Skip temporary files created by RPM. */
> + if (memchr (name, len, ';') != NULL)
> + return true;
> + /* Skip temporary files created by dpkg. */
> + if (len > 4 && memcmp (name + len - 4, ".tmp", 4) == 0)
Wouldn't this potentially read out the bounds for sizes [5, 8)? Maybe
just use strcmp?
> + return true;
> + return false;
> +}
>
> static void
> search_dir (const struct dir_entry *entry)
> @@ -711,18 +736,8 @@ search_dir (const struct dir_entry *entry)
> continue;
>
> size_t len = strlen (direntry->d_name);
> - /* Skip temporary files created by the prelink program. Files with
> - names like these are never really DSOs we want to look at. */
> - if (len >= sizeof (".#prelink#") - 1)
> - {
> - if (strcmp (direntry->d_name + len - sizeof (".#prelink#") + 1,
> - ".#prelink#") == 0)
> - continue;
> - if (len >= sizeof (".#prelink#.XXXXXX") - 1
> - && memcmp (direntry->d_name + len - sizeof (".#prelink#.XXXXXX")
> - + 1, ".#prelink#.", sizeof (".#prelink#.") - 1) == 0)
> - continue;
> - }
> + if (skip_dso_based_on_name (direntry->d_name, len))
> + continue;
> if (asprintf (&file_name, "%s/%s", entry->path, direntry->d_name) < 0)
> error (EXIT_FAILURE, errno, _("Could not form library path"));
> if (opt_chroot != NULL)
>
> base-commit: f5677d9cebb12edcd9301dbb5cf40f82618b46af
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] elf: ldconfig should skip temporary files created by package managers
2023-10-20 12:48 ` Adhemerval Zanella Netto
@ 2023-10-20 14:41 ` Florian Weimer
2023-10-20 15:15 ` Adhemerval Zanella Netto
0 siblings, 1 reply; 12+ messages in thread
From: Florian Weimer @ 2023-10-20 14:41 UTC (permalink / raw)
To: Adhemerval Zanella Netto; +Cc: libc-alpha
* Adhemerval Zanella Netto:
> On 20/10/23 09:29, Florian Weimer wrote:
>> This avoids crashes due to partially written files, after a package
>> update is interrupted.
>
> It seems kinda unreliable a tool create temporary files on the libdir,
> although we alrady handle for prelinkk so it makes sense to extend to
> other program as well. Maybe another option would to proper parse the
> file header and only handle ELF files, so ldconfig is not bounded to
> name patterns.
The files look like ELF files, but a truncated, so that we get SIGBUS
because the mapping we access is only halfway there.
>> + /* Skip temporary files created by dpkg. */
>> + if (len > 4 && memcmp (name + len - 4, ".tmp", 4) == 0)
>
> Wouldn't this potentially read out the bounds for sizes [5, 8)? Maybe
> just use strcmp?
I don't see how.
ABCDE
^ name + len
^ name + len - 4
.tmp
Looks okay to me?
Thanks,
Florian
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] elf: ldconfig should skip temporary files created by package managers
2023-10-20 14:41 ` Florian Weimer
@ 2023-10-20 15:15 ` Adhemerval Zanella Netto
0 siblings, 0 replies; 12+ messages in thread
From: Adhemerval Zanella Netto @ 2023-10-20 15:15 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha
On 20/10/23 11:41, Florian Weimer wrote:
> * Adhemerval Zanella Netto:
>
>> On 20/10/23 09:29, Florian Weimer wrote:
>>> This avoids crashes due to partially written files, after a package
>>> update is interrupted.
>>
>> It seems kinda unreliable a tool create temporary files on the libdir,
>> although we alrady handle for prelinkk so it makes sense to extend to
>> other program as well. Maybe another option would to proper parse the
>> file header and only handle ELF files, so ldconfig is not bounded to
>> name patterns.
>
> The files look like ELF files, but a truncated, so that we get SIGBUS
> because the mapping we access is only halfway there.
Right, filtering by the name indeed seems less troublesome.
>
>>> + /* Skip temporary files created by dpkg. */
>>> + if (len > 4 && memcmp (name + len - 4, ".tmp", 4) == 0)
>>
>> Wouldn't this potentially read out the bounds for sizes [5, 8)? Maybe
>> just use strcmp?
>
> I don't see how.
>
> ABCDE
> ^ name + len
> ^ name + len - 4
> .tmp
>
> Looks okay to me?
Yeah, you are right.
LGTM, thanks.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] elf: ldconfig should skip temporary files created by package managers
2023-10-20 12:29 [PATCH] elf: ldconfig should skip temporary files created by package managers Florian Weimer
2023-10-20 12:48 ` Adhemerval Zanella Netto
@ 2023-10-20 21:33 ` Guillem Jover
2023-10-21 22:14 ` Florian Weimer
2023-10-21 11:37 ` Cristian Rodríguez
2023-11-14 23:34 ` DJ Delorie
3 siblings, 1 reply; 12+ messages in thread
From: Guillem Jover @ 2023-10-20 21:33 UTC (permalink / raw)
To: libc-alpha; +Cc: Florian Weimer
Hi!
[ Hope the reply works, had to construct it from the web archive. ]
On Fri Oct 20 12:29:50 GMT 2023 Florian Weimer wrote:
> diff --git a/elf/ldconfig.c b/elf/ldconfig.c
> index d26eef1fb4..02387a169c 100644
> --- a/elf/ldconfig.c
> +++ b/elf/ldconfig.c
> @@ -661,6 +661,31 @@ struct dlib_entry
> struct dlib_entry *next;
> };
>
> +/* Skip some temporary DSO files. These files may be partially written
> + and lead to ldconfig crashes when examined. */
> +static bool
> +skip_dso_based_on_name (const char *name, size_t len)
> +{
> + /* Skip temporary files created by the prelink program. Files with
> + names like these are never really DSOs we want to look at. */
> + if (len >= sizeof (".#prelink#") - 1)
> + {
> + if (strcmp (name + len - sizeof (".#prelink#") + 1,
> + ".#prelink#") == 0)
> + return true;
> + if (len >= sizeof (".#prelink#.XXXXXX") - 1
> + && memcmp (name + len - sizeof (".#prelink#.XXXXXX")
> + + 1, ".#prelink#.", sizeof (".#prelink#.") - 1) == 0)
> + return true;
> + }
> + /* Skip temporary files created by RPM. */
> + if (memchr (name, len, ';') != NULL)
> + return true;
> + /* Skip temporary files created by dpkg. */
> + if (len > 4 && memcmp (name + len - 4, ".tmp", 4) == 0)
> + return true;
If this refers to files dpkg creates during an unpack, then for
regular files, it might end up creating ones ending with «.dpkg-new»
and «.dpkg-tmp» (for conffiles there are other extensions used, but
those would not seem relevant here).
(And, hmm, I should probably try to document that somewhere in the
dpkg manual pages or docs.)
> + return false;
> +}
>
> static void
> search_dir (const struct dir_entry *entry)
Thanks,
Guillem
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] elf: ldconfig should skip temporary files created by package managers
2023-10-20 21:33 ` Guillem Jover
@ 2023-10-21 22:14 ` Florian Weimer
0 siblings, 0 replies; 12+ messages in thread
From: Florian Weimer @ 2023-10-21 22:14 UTC (permalink / raw)
To: Guillem Jover; +Cc: libc-alpha
* Guillem Jover:
>> + /* Skip temporary files created by dpkg. */
>> + if (len > 4 && memcmp (name + len - 4, ".tmp", 4) == 0)
>> + return true;
>
> If this refers to files dpkg creates during an unpack, then for
> regular files, it might end up creating ones ending with «.dpkg-new»
> and «.dpkg-tmp» (for conffiles there are other extensions used, but
> those would not seem relevant here).
Ugh, I must have been momentarily confused when I made this change.
I did look at strace out, but apparently promptly forget what I saw
(although in my test, only .dpkg-new showed up).
I'll sent a follow-up fix.
Thanks,
Florian
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] elf: ldconfig should skip temporary files created by package managers
2023-10-20 12:29 [PATCH] elf: ldconfig should skip temporary files created by package managers Florian Weimer
2023-10-20 12:48 ` Adhemerval Zanella Netto
2023-10-20 21:33 ` Guillem Jover
@ 2023-10-21 11:37 ` Cristian Rodríguez
2023-10-21 22:20 ` Florian Weimer
2023-11-14 23:34 ` DJ Delorie
3 siblings, 1 reply; 12+ messages in thread
From: Cristian Rodríguez @ 2023-10-21 11:37 UTC (permalink / raw)
To: libc-alpha
[-- Attachment #1: Type: text/plain, Size: 249 bytes --]
On Fri, Oct 20, 2023 at 9:30 AM Florian Weimer <fweimer@redhat.com> wrote:
> This avoids crashes due to partially written files, after a package
> update is interrupted.
>
Do you know if rpm at least as been fixed not to do that ?
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] elf: ldconfig should skip temporary files created by package managers
2023-10-21 11:37 ` Cristian Rodríguez
@ 2023-10-21 22:20 ` Florian Weimer
0 siblings, 0 replies; 12+ messages in thread
From: Florian Weimer @ 2023-10-21 22:20 UTC (permalink / raw)
To: Cristian Rodríguez; +Cc: libc-alpha
* Cristian Rodríguez:
> On Fri, Oct 20, 2023 at 9:30 AM Florian Weimer <fweimer@redhat.com> wrote:
>
>> This avoids crashes due to partially written files, after a package
>> update is interrupted.
>
> Do you know if rpm at least as been fixed not to do that ?
I think DNF/RPM nowadays intercept SIGINT during file writing, but
signals are enabled when scriptlet execution starts, so you probably
still get a corrupted system when you hit ^C.
I don't think there are plans to use O_TMPFILE to prevent partial files
from showing up. So if there is a system crash, partial files could
still be left behind even if RPM is not interrupted so easily anymore.
Thanks,
Florian
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] elf: ldconfig should skip temporary files created by package managers
2023-10-20 12:29 [PATCH] elf: ldconfig should skip temporary files created by package managers Florian Weimer
` (2 preceding siblings ...)
2023-10-21 11:37 ` Cristian Rodríguez
@ 2023-11-14 23:34 ` DJ Delorie
2023-11-15 5:02 ` Florian Weimer
3 siblings, 1 reply; 12+ messages in thread
From: DJ Delorie @ 2023-11-14 23:34 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha
Florian Weimer <fweimer@redhat.com> writes:
> + /* Skip temporary files created by RPM. */
> + if (memchr (name, len, ';') != NULL)
> + return true;
Should be memchr (name, ';', len)
This caused ldconfig to miss many symlinks during my latest rawhide
update, making the system temporarily unbootable...
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] elf: ldconfig should skip temporary files created by package managers
2023-11-14 23:34 ` DJ Delorie
@ 2023-11-15 5:02 ` Florian Weimer
2023-11-15 6:39 ` Florian Weimer
0 siblings, 1 reply; 12+ messages in thread
From: Florian Weimer @ 2023-11-15 5:02 UTC (permalink / raw)
To: DJ Delorie; +Cc: libc-alpha
* DJ Delorie:
> Florian Weimer <fweimer@redhat.com> writes:
>> + /* Skip temporary files created by RPM. */
>> + if (memchr (name, len, ';') != NULL)
>> + return true;
>
> Should be memchr (name, ';', len)
>
> This caused ldconfig to miss many symlinks during my latest rawhide
> update, making the system temporarily unbootable...
Yeah, sorry, patch is here:
[PATCH v2] ldconfig: Fixes for skipping temporary files.
<https://inbox.sourceware.org/libc-alpha/87v8a83f9t.fsf@oldenburg.str.redhat.com/>
It's already fixed in the Fedora rawhide sources, but the build with the
fix applied triggered another qsort regression, so it's not been pushed
to the repositories yet.
Thanks,
Florian
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] elf: ldconfig should skip temporary files created by package managers
2023-11-15 5:02 ` Florian Weimer
@ 2023-11-15 6:39 ` Florian Weimer
2023-11-15 16:18 ` DJ Delorie
0 siblings, 1 reply; 12+ messages in thread
From: Florian Weimer @ 2023-11-15 6:39 UTC (permalink / raw)
To: DJ Delorie; +Cc: libc-alpha
* Florian Weimer:
> * DJ Delorie:
>
>> Florian Weimer <fweimer@redhat.com> writes:
>>> + /* Skip temporary files created by RPM. */
>>> + if (memchr (name, len, ';') != NULL)
>>> + return true;
>>
>> Should be memchr (name, ';', len)
>>
>> This caused ldconfig to miss many symlinks during my latest rawhide
>> update, making the system temporarily unbootable...
>
> Yeah, sorry, patch is here:
>
> [PATCH v2] ldconfig: Fixes for skipping temporary files.
> <https://inbox.sourceware.org/libc-alpha/87v8a83f9t.fsf@oldenburg.str.redhat.com/>
>
> It's already fixed in the Fedora rawhide sources, but the build with the
> fix applied triggered another qsort regression, so it's not been pushed
> to the repositories yet.
And: Any idea why this even matters? Why can't we find those shared
objects in /usr/lib64 directly, without the cache?
Thanks,
Florian
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] elf: ldconfig should skip temporary files created by package managers
2023-11-15 6:39 ` Florian Weimer
@ 2023-11-15 16:18 ` DJ Delorie
0 siblings, 0 replies; 12+ messages in thread
From: DJ Delorie @ 2023-11-15 16:18 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha
Florian Weimer <fweimer@redhat.com> writes:
> And: Any idea why this even matters? Why can't we find those shared
> objects in /usr/lib64 directly, without the cache?
It's not the cache that's missing, it's the symlink itself:
lrwxrwxrwx. 1 root root 16 Nov 14 16:58 /lib64/libkmod.so.2 -> libkmod.so.2.4.1
-rwxr-xr-x. 1 root root 115960 Nov 8 19:00 /lib64/libkmod.so.2.4.1
The broken case looked like this:
-rwxr-xr-x. 1 root root 115960 Nov 8 19:00 /lib64/libkmod.so.2.4.1
so libkmod.so.2 *didn't exist*.
It was just my luck that libkmod was upgraded from 2.4.0 to 2.4.1, so
the symlink was needed, *and* libkmod is a boot-critical shared object,
*and* the bug caused that one to be skipped.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2023-11-15 17:55 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-20 12:29 [PATCH] elf: ldconfig should skip temporary files created by package managers Florian Weimer
2023-10-20 12:48 ` Adhemerval Zanella Netto
2023-10-20 14:41 ` Florian Weimer
2023-10-20 15:15 ` Adhemerval Zanella Netto
2023-10-20 21:33 ` Guillem Jover
2023-10-21 22:14 ` Florian Weimer
2023-10-21 11:37 ` Cristian Rodríguez
2023-10-21 22:20 ` Florian Weimer
2023-11-14 23:34 ` DJ Delorie
2023-11-15 5:02 ` Florian Weimer
2023-11-15 6:39 ` Florian Weimer
2023-11-15 16:18 ` DJ Delorie
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 read-only IMAP folder(s) and NNTP newsgroup(s).