elf_strptr may attempt to call __libelf_decompress_elf while holding a
rdlock. This will cause a deadlock since __libelf_decompress_elf
calls elf_getdata which attempts to acquire the corresponding wrlock.
Additionally, elf_strptr.c:get_zdata modifies its Elf_Scn argument
while only holding a rdlock when a wrlock is needed.
Fix this by making elf_strptr acquire the wrlock while decompressing
and by adding a __libelf_decompress_elf_wrlock function that assumes
the caller is already holding a wrlock.
libelf/
* elf_compress.c (decompress_elf): Renamed from
__libelf_decompress_elf with new parameter wrlocked indicating
whether the function was called with a wrlock held. Also calls
_wrlock variants of internal libelf functions when appropriate.
(__libelf_decompress_elf): Now calls decompress_elf
with wrlocked argument indicating no wrlock is held.
(__libelf_decompress_elf_wrlock): New function that
calls decompress_elf with wrlocked argument indicating a wrlock is
held.
* elf_getdata.c (getdata): Renamed from __elf_getdata_rdlock with
new wrlocked parameter indicating whether the function was called
with a wrlock held.
(__elf_getdata_rdlock): Now calls getdata with wrlocked
argument indicating no wrlock is held.
(__elf_getdata_wrlock): Now calls getdata with wrlocked
argument indicating a wrlock is held.
* elf_strptr.c (get_zdata_wrlock): Renamed from get_zdata. Now assumes
a wrlock is held when called.
(elf_strptr): Upgrades rdlock to wrlock before calling
get_zdata_wrlock.
* libelfP.h (__libelf_decompress_elf_wrlock): Add declaration.
Signed-off-by: Aaron Merey <[email protected]>
---
libelf/elf_compress.c | 45 +++++++++++++++++++++++++++++++++++++------
libelf/elf_getdata.c | 41 ++++++++++++++++++++-------------------
libelf/elf_strptr.c | 43 +++++++++++++++++++++++++++++++++--------
libelf/libelfP.h | 3 +++
4 files changed, 98 insertions(+), 34 deletions(-)
diff --git a/libelf/elf_compress.c b/libelf/elf_compress.c
index b982510d..bb341dda 100644
--- a/libelf/elf_compress.c
+++ b/libelf/elf_compress.c
@@ -465,13 +465,30 @@ __libelf_decompress (int chtype, void *buf_in, size_t
size_in, size_t size_out)
}
}
-void *
-internal_function
-__libelf_decompress_elf (Elf_Scn *scn, size_t *size_out, size_t *addralign)
+static void *
+decompress_elf (Elf_Scn *scn, size_t *size_out, size_t *addralign,
+ bool wrlocked)
{
GElf_Chdr chdr;
- if (gelf_getchdr (scn, &chdr) == NULL)
- return NULL;
+
+ if (scn->elf->class == ELFCLASS32)
+ {
+ Elf32_Chdr *c
+ = wrlocked ? __elf32_getchdr_wrlock (scn) : elf32_getchdr (scn);
+ if (c == NULL)
+ return NULL;
+ chdr.ch_type = c->ch_type;
+ chdr.ch_size = c->ch_size;
+ chdr.ch_addralign = c->ch_addralign;
+ }
+ else
+ {
+ Elf64_Chdr *c
+ = wrlocked ? __elf64_getchdr_wrlock (scn) : elf64_getchdr (scn);
+ if (c == NULL)
+ return NULL;
+ chdr = *c;
+ }
bool unknown_compression = false;
if (chdr.ch_type != ELFCOMPRESS_ZLIB)
@@ -503,7 +520,8 @@ __libelf_decompress_elf (Elf_Scn *scn, size_t *size_out,
size_t *addralign)
is slightly inefficient when the raw data needs to be
converted since then we'll be converting the whole buffer and
not just Chdr. */
- Elf_Data *data = elf_getdata (scn, NULL);
+ Elf_Data *data
+ = wrlocked ? __elf_getdata_wrlock (scn, NULL) : elf_getdata (scn, NULL);
if (data == NULL)
return NULL;
@@ -520,6 +538,21 @@ __libelf_decompress_elf (Elf_Scn *scn, size_t *size_out,
size_t *addralign)
return buf_out;
}
+void *
+internal_function
+__libelf_decompress_elf_wrlock (Elf_Scn *scn, size_t *size_out,
+ size_t *addralign)
+{
+ return decompress_elf (scn, size_out, addralign, true);
+}
+
+void *
+internal_function
+__libelf_decompress_elf (Elf_Scn *scn, size_t *size_out, size_t *addralign)
+{
+ return decompress_elf (scn, size_out, addralign, false);
+}
+
/* Assumes buf is a malloced buffer. */
void
internal_function
diff --git a/libelf/elf_getdata.c b/libelf/elf_getdata.c
index 7c3ac043..fc66684b 100644
--- a/libelf/elf_getdata.c
+++ b/libelf/elf_getdata.c
@@ -471,13 +471,11 @@ __libelf_set_data_list_rdlock (Elf_Scn *scn, int wrlocked)
scn->data_list_rear = &scn->data_list;
}
-Elf_Data *
-internal_function
-__elf_getdata_rdlock (Elf_Scn *scn, Elf_Data *data)
+static Elf_Data *
+getdata (Elf_Scn *scn, Elf_Data *data, int wrlocked)
{
Elf_Data *result = NULL;
Elf *elf;
- int locked = 0;
if (scn == NULL)
return NULL;
@@ -539,13 +537,16 @@ __elf_getdata_rdlock (Elf_Scn *scn, Elf_Data *data)
/* If the data for this section was not yet initialized do it now. */
if (scn->data_read == 0)
{
- /* We cannot acquire a write lock while we are holding a read
- lock. Therefore give up the read lock and then get the write
- lock. But this means that the data could meanwhile be
- modified, therefore start the tests again. */
- rwlock_unlock (elf->lock);
- rwlock_wrlock (elf->lock);
- locked = 1;
+ if (wrlocked == 0)
+ {
+ /* We cannot acquire a write lock while we are holding a read
+ lock. Therefore give up the read lock and then get the write
+ lock. But this means that the data could meanwhile be
+ modified, therefore start the tests again. */
+ rwlock_unlock (elf->lock);
+ rwlock_wrlock (elf->lock);
+ wrlocked = 1;
+ }
/* Read the data from the file. There is always a file (or
memory region) associated with this descriptor since
@@ -559,7 +560,7 @@ __elf_getdata_rdlock (Elf_Scn *scn, Elf_Data *data)
empty in case the section has size zero (for whatever reason).
Now create the converted data in case this is necessary. */
if (scn->data_list_rear == NULL)
- __libelf_set_data_list_rdlock (scn, locked);
+ __libelf_set_data_list_rdlock (scn, wrlocked);
/* Return the first data element in the list. */
result = &scn->data_list.data.d;
@@ -585,15 +586,15 @@ elf_getdata (Elf_Scn *scn, Elf_Data *data)
Elf_Data *
internal_function
-__elf_getdata_wrlock (Elf_Scn *scn, Elf_Data *data)
+__elf_getdata_rdlock (Elf_Scn *scn, Elf_Data *data)
{
- Elf_Data *result;
-
- if (scn == NULL)
- return NULL;
-
- result = __elf_getdata_rdlock (scn, data);
+ return getdata (scn, data, 0);
+}
- return result;
+Elf_Data *
+internal_function
+__elf_getdata_wrlock (Elf_Scn *scn, Elf_Data *data)
+{
+ return getdata (scn, data, 1);
}
INTDEF(elf_getdata)
diff --git a/libelf/elf_strptr.c b/libelf/elf_strptr.c
index 7be7f5e8..f0a7d7ea 100644
--- a/libelf/elf_strptr.c
+++ b/libelf/elf_strptr.c
@@ -40,10 +40,10 @@
static void *
-get_zdata (Elf_Scn *strscn)
+get_zdata_wrlock (Elf_Scn *strscn)
{
size_t zsize, zalign;
- void *zdata = __libelf_decompress_elf (strscn, &zsize, &zalign);
+ void *zdata = __libelf_decompress_elf_wrlock (strscn, &zsize, &zalign);
if (zdata == NULL)
return NULL;
@@ -100,6 +100,7 @@ elf_strptr (Elf *elf, size_t idx, size_t offset)
}
}
+ int wrlocked = 0;
size_t sh_size = 0;
if (elf->class == ELFCLASS32)
{
@@ -115,8 +116,19 @@ elf_strptr (Elf *elf, size_t idx, size_t offset)
sh_size = shdr->sh_size;
else
{
- if (strscn->zdata_base == NULL && get_zdata (strscn) == NULL)
- goto out;
+ if (strscn->zdata_base == NULL)
+ {
+ rwlock_unlock (elf->lock);
+ rwlock_wrlock (elf->lock);
+ wrlocked = 1;
+
+ /* Skip decompression if it occurred while grabbing
+ the wrlock. */
+ if (strscn->zdata_base == NULL
+ && get_zdata_wrlock (strscn) == NULL)
+ goto out;
+ }
+
sh_size = strscn->zdata_size;
}
@@ -141,8 +153,19 @@ elf_strptr (Elf *elf, size_t idx, size_t offset)
sh_size = shdr->sh_size;
else
{
- if (strscn->zdata_base == NULL && get_zdata (strscn) == NULL)
- goto out;
+ if (strscn->zdata_base == NULL)
+ {
+ rwlock_unlock (elf->lock);
+ rwlock_wrlock (elf->lock);
+ wrlocked = 1;
+
+ /* Skip decompression if it occurred while grabbing
+ the wrlock. */
+ if (strscn->zdata_base == NULL
+ && get_zdata_wrlock (strscn) == NULL)
+ goto out;
+ }
+
sh_size = strscn->zdata_size;
}
@@ -156,8 +179,12 @@ elf_strptr (Elf *elf, size_t idx, size_t offset)
if (strscn->rawdata_base == NULL && ! strscn->data_read)
{
- rwlock_unlock (elf->lock);
- rwlock_wrlock (elf->lock);
+ if (wrlocked == 0)
+ {
+ rwlock_unlock (elf->lock);
+ rwlock_wrlock (elf->lock);
+ wrlocked = 1;
+ }
if (strscn->rawdata_base == NULL && ! strscn->data_read
/* Read the section data. */
&& __libelf_set_rawdata_wrlock (strscn) != 0)
diff --git a/libelf/libelfP.h b/libelf/libelfP.h
index 2403d796..52767aa5 100644
--- a/libelf/libelfP.h
+++ b/libelf/libelfP.h
@@ -602,6 +602,9 @@ extern void * __libelf_decompress (int chtype, void
*buf_in, size_t size_in,
extern void * __libelf_decompress_elf (Elf_Scn *scn,
size_t *size_out, size_t *addralign)
internal_function;
+extern void * __libelf_decompress_elf_wrlock (Elf_Scn *scn, size_t *size_out,
+ size_t *addralign)
+ internal_function;
extern void __libelf_reset_rawdata (Elf_Scn *scn, void *buf, size_t size,
--
2.55.0
