[2/3] dl-load: enforce soname uniqueness of loaded libraries
Commit Message
When loading a new library of name `libname`, the loader currently
checks if any of the already loaded libraries have the soname `libname`.
If so, it returns the already loaded library of soname `libname`. If
not, it checks for the corresponding file `libname` on disk and compares
the device and inode number of already loaded libraries. If there is a
match, it returns the already loaded library of the same inode number.
If not, it proceeds to load the library of path is `libname`.
This patch adds the additional check of comparing the soname of the
newly loaded library with the sonames of already loaded libraries.
This new behavior enforces that no two loaded libaries have the same
soname, bringing a sane property in the system.
Note that this edge case can be encountered during a system upgrade, or
when using certain configuration of file system mounts, where inodes
cannot be relied upon.
---
elf/dl-load.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)
Comments
On 1/6/20 10:44 AM, Nicolas Viennot wrote:
> When loading a new library of name `libname`, the loader currently
> checks if any of the already loaded libraries have the soname `libname`.
> If so, it returns the already loaded library of soname `libname`. If
> not, it checks for the corresponding file `libname` on disk and compares
> the device and inode number of already loaded libraries. If there is a
> match, it returns the already loaded library of the same inode number.
> If not, it proceeds to load the library of path is `libname`.
>
> This patch adds the additional check of comparing the soname of the
> newly loaded library with the sonames of already loaded libraries.
>
> This new behavior enforces that no two loaded libaries have the same
> soname, bringing a sane property in the system.
>
> Note that this edge case can be encountered during a system upgrade, or
> when using certain configuration of file system mounts, where inodes
> cannot be relied upon.
Nicolas,
Thank you for working on this! This does look like an interesting patch
and provides belt-and-suspenders and in general making the loader more
robust against such odd failures is a good idea. We often see such things
as crashes when systems are being live updated by rpm/dpkg etc.
Before we can accept any contributions we need a copyright assignment
from you for glibc. This is handled by the FSF.
"FSF copyright Assignment"
https://sourceware.org/glibc/wiki/Contribution%20checklist#FSF_copyright_Assignment
I suggest the futures assignment:
http://git.savannah.gnu.org/cgit/gnulib.git/plain/doc/Copyright/request-assign.future
This would allow us to accept this patch and all subsequent patches
you would want to submit. Given that you're submitting from a corporate
email it may require you to check to see if your employer owns the
copyright of your work, in which case they would need to sign a corporate
assignment. The FSF will help you through this process, and we accept
digital assignments now so the process is faster than ever before.
Please also have a look at the "Contribution Checklist":
https://sourceware.org/glibc/wiki/Contribution%20checklist
Thank you again for your patches! I look forward to your contributions!
Hello Carlos,
I wanted to let you know that the copyright issue has not been solved between my employer and GNU.
I have exhausted the amount of time I want to spend on this issue, so I am abandoning the patches.
Thank you for the time you spent into this.
Nico.
-----Original Message-----
From: Carlos O'Donell <codonell@redhat.com>
Sent: Saturday, February 1, 2020 12:49 AM
To: Nicolas Viennot <Nicolas.Viennot@twosigma.com>; libc-alpha@sourceware.org
Subject: Re: [PATCH 2/3] dl-load: enforce soname uniqueness of loaded libraries
On 1/6/20 10:44 AM, Nicolas Viennot wrote:
> When loading a new library of name `libname`, the loader currently
> checks if any of the already loaded libraries have the soname `libname`.
> If so, it returns the already loaded library of soname `libname`. If
> not, it checks for the corresponding file `libname` on disk and
> compares the device and inode number of already loaded libraries. If
> there is a match, it returns the already loaded library of the same inode number.
> If not, it proceeds to load the library of path is `libname`.
>
> This patch adds the additional check of comparing the soname of the
> newly loaded library with the sonames of already loaded libraries.
>
> This new behavior enforces that no two loaded libaries have the same
> soname, bringing a sane property in the system.
>
> Note that this edge case can be encountered during a system upgrade,
> or when using certain configuration of file system mounts, where
> inodes cannot be relied upon.
Nicolas,
Thank you for working on this! This does look like an interesting patch and provides belt-and-suspenders and in general making the loader more robust against such odd failures is a good idea. We often see such things as crashes when systems are being live updated by rpm/dpkg etc.
Before we can accept any contributions we need a copyright assignment from you for glibc. This is handled by the FSF.
"FSF copyright Assignment"
https://sourceware.org/glibc/wiki/Contribution%20checklist#FSF_copyright_Assignment
I suggest the futures assignment:
http://git.savannah.gnu.org/cgit/gnulib.git/plain/doc/Copyright/request-assign.future
This would allow us to accept this patch and all subsequent patches you would want to submit. Given that you're submitting from a corporate email it may require you to check to see if your employer owns the copyright of your work, in which case they would need to sign a corporate assignment. The FSF will help you through this process, and we accept digital assignments now so the process is faster than ever before.
Please also have a look at the "Contribution Checklist":
https://sourceware.org/glibc/wiki/Contribution%20checklist
Thank you again for your patches! I look forward to your contributions!
--
Cheers,
Carlos.
Nicolas,
Sorry to hear that. If you have any feedback you wish to share either
publicly or privately about the process, please don't hesitate to reach
out.
On 6/9/20 4:31 PM, Nicolas Viennot wrote:
> Hello Carlos,
>
> I wanted to let you know that the copyright issue has not been solved between my employer and GNU.
> I have exhausted the amount of time I want to spend on this issue, so I am abandoning the patches.
> Thank you for the time you spent into this.
>
> Nico.
>
> -----Original Message-----
> From: Carlos O'Donell <codonell@redhat.com>
> Sent: Saturday, February 1, 2020 12:49 AM
> To: Nicolas Viennot <Nicolas.Viennot@twosigma.com>; libc-alpha@sourceware.org
> Subject: Re: [PATCH 2/3] dl-load: enforce soname uniqueness of loaded libraries
>
> On 1/6/20 10:44 AM, Nicolas Viennot wrote:
>> When loading a new library of name `libname`, the loader currently
>> checks if any of the already loaded libraries have the soname `libname`.
>> If so, it returns the already loaded library of soname `libname`. If
>> not, it checks for the corresponding file `libname` on disk and
>> compares the device and inode number of already loaded libraries. If
>> there is a match, it returns the already loaded library of the same inode number.
>> If not, it proceeds to load the library of path is `libname`.
>>
>> This patch adds the additional check of comparing the soname of the
>> newly loaded library with the sonames of already loaded libraries.
>>
>> This new behavior enforces that no two loaded libaries have the same
>> soname, bringing a sane property in the system.
>>
>> Note that this edge case can be encountered during a system upgrade,
>> or when using certain configuration of file system mounts, where
>> inodes cannot be relied upon.
>
> Nicolas,
>
> Thank you for working on this! This does look like an interesting patch and provides belt-and-suspenders and in general making the loader more robust against such odd failures is a good idea. We often see such things as crashes when systems are being live updated by rpm/dpkg etc.
>
> Before we can accept any contributions we need a copyright assignment from you for glibc. This is handled by the FSF.
>
> "FSF copyright Assignment"
> https://sourceware.org/glibc/wiki/Contribution%20checklist#FSF_copyright_Assignment
>
> I suggest the futures assignment:
> http://git.savannah.gnu.org/cgit/gnulib.git/plain/doc/Copyright/request-assign.future
>
> This would allow us to accept this patch and all subsequent patches you would want to submit. Given that you're submitting from a corporate email it may require you to check to see if your employer owns the copyright of your work, in which case they would need to sign a corporate assignment. The FSF will help you through this process, and we accept digital assignments now so the process is faster than ever before.
>
> Please also have a look at the "Contribution Checklist":
> https://sourceware.org/glibc/wiki/Contribution%20checklist
>
> Thank you again for your patches! I look forward to your contributions!
>
> --
> Cheers,
> Carlos.
>
@@ -851,6 +851,8 @@ lose (int code, int fd, const char *name, char *realname, struct link_map *l,
_dl_signal_error (code, name, NULL, msg);
}
+static struct link_map *
+_dl_find_object_from_name(const char *name, Lmid_t nsid);
/* Map in the shared object NAME, actually located in REALNAME, and already
opened on FD. */
@@ -1202,6 +1204,52 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
elf_get_dynamic_info (l, NULL);
+ /*
+ * Check if we have a loaded object with the same soname. If that's the
+ * case, return the previously loaded object. The loop follows a similar
+ * logic compared to the beginning of _dl_map_object().
+ * the loader assumes that the sonames of loaded objects are without
+ * duplicates.
+ */
+
+ if (l->l_info[DT_SONAME] != NULL)
+ {
+ static struct link_map *_l;
+ const char *soname;
+
+ soname = (((const char *) D_PTR (l, l_info[DT_STRTAB])
+ + l->l_info[DT_SONAME]->d_un.d_val));
+
+ _l = _dl_find_object_from_name(soname, nsid);
+ if (__glibc_unlikely(_l != NULL))
+ {
+ /* Free current object */
+ _dl_unmap_segments (l);
+
+ if (!l->l_libname->dont_free)
+ free (l->l_libname);
+
+ if (l->l_phdr_allocated)
+ free ((void *) l->l_phdr);
+
+ __close_nocancel (fd);
+ if (l->l_origin != (char *) -1l)
+ free ((char *) l->l_origin);
+ free (l);
+ free (realname);
+
+ if (make_consistent)
+ {
+ r->r_state = RT_CONSISTENT;
+ _dl_debug_state ();
+ }
+
+ /* Cache the name and return the previously loaded object */
+ add_name_to_object (_l, name);
+ return _l;
+ }
+ }
+
/* Make sure we are not dlopen'ing an object that has the
DF_1_NOOPEN flag set, or a PIE object. */
if ((__glibc_unlikely (l->l_flags_1 & DF_1_NOOPEN)