Message ID | 8a449dd92e5f41189f95abbe803c82e3@EXMBDFT10.ad.twosigma.com |
---|---|
State | New |
Headers | show |
Series | [1/3] dl-load: extract _dl_find_object_from_name() from _dl_map_object() | expand |
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. >
diff --git a/elf/dl-load.c b/elf/dl-load.c index c436a447af..c5c997f95b 100644 --- a/elf/dl-load.c +++ b/elf/dl-load.c @@ -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)