From: Tobias Stoeckmann <tobias@stoeckmann.org>
To: linux-modules@vger.kernel.org
Subject: [PATCH] libkmod: check size in elf_get_mem
Date: Tue, 10 Feb 2015 20:38:03 +0100 [thread overview]
Message-ID: <20150210193803.GA11559@localhost> (raw)
Hi,
the function elf_get_mem properly checks if offset is smaller than elf->size,
but does not perform further memory size validations.
Supply desired size as third argument to elf_get_mem, which is in sync
with elf_get_uint/elf_set_uint then.
As you can see, further unbounded string operations like strlen are still
prone to lead to out-of-bounds access, which will be covered in another patch.
Tobias
---
libkmod/libkmod-elf.c | 52 ++++++++++++++++++++++++++++++++-------------------
1 file changed, 33 insertions(+), 19 deletions(-)
diff --git a/libkmod/libkmod-elf.c b/libkmod/libkmod-elf.c
index 2f50ad2..85ba681 100644
--- a/libkmod/libkmod-elf.c
+++ b/libkmod/libkmod-elf.c
@@ -197,12 +197,15 @@ static inline int elf_set_uint(struct kmod_elf *elf, uint64_t offset, uint64_t s
return 0;
}
-static inline const void *elf_get_mem(const struct kmod_elf *elf, uint64_t offset)
+static inline const void *elf_get_mem(const struct kmod_elf *elf, uint64_t offset, uint64_t size)
{
- assert(offset < elf->size);
- if (offset >= elf->size) {
- ELFDBG(elf, "out-of-bounds: %"PRIu64" >= %"PRIu64" (ELF size)\n",
- offset, elf->size);
+ uint64_t end;
+
+ assert(!addu64_overflow(offset, size, &end));
+ assert(end <= elf->size);
+ if (addu64_overflow(offset, size, &end) || end > elf->size) {
+ ELFDBG(elf, "out-of-bounds: %"PRIu64" + %"PRIu64" > %"PRIu64" (ELF size)\n",
+ offset, size, elf->size);
return NULL;
}
return elf->memory + offset;
@@ -218,7 +221,8 @@ static inline const void *elf_get_section_header(const struct kmod_elf *elf, uin
return NULL;
}
return elf_get_mem(elf, elf->header.section.offset +
- idx * elf->header.section.entry_size);
+ idx * elf->header.section.entry_size,
+ elf->header.section.entry_size);
}
static inline int elf_get_section_info(const struct kmod_elf *elf, uint16_t idx, uint64_t *offset, uint64_t *size, uint32_t *nameoff)
@@ -266,7 +270,8 @@ static inline int elf_get_section_info(const struct kmod_elf *elf, uint16_t idx,
static const char *elf_get_strings_section(const struct kmod_elf *elf, uint64_t *size)
{
*size = elf->header.strings.size;
- return elf_get_mem(elf, elf->header.strings.offset);
+ return elf_get_mem(elf, elf->header.strings.offset,
+ elf->header.strings.size);
}
struct kmod_elf *kmod_elf_new(const void *memory, off_t size)
@@ -307,11 +312,13 @@ struct kmod_elf *kmod_elf_new(const void *memory, off_t size)
elf->header.strings.section = READV(e_shstrndx); \
elf->header.machine = READV(e_machine)
if (elf->class & KMOD_ELF_32) {
- const Elf32_Ehdr *hdr _unused_ = elf_get_mem(elf, 0);
+ size_t hdr_size = sizeof(Elf32_Ehdr);
+ const Elf32_Ehdr *hdr _unused_ = elf_get_mem(elf, 0, hdr_size);
LOAD_HEADER;
shdr_size = sizeof(Elf32_Shdr);
} else {
- const Elf64_Ehdr *hdr _unused_ = elf_get_mem(elf, 0);
+ size_t hdr_size = sizeof(Elf64_Ehdr);
+ const Elf64_Ehdr *hdr _unused_ = elf_get_mem(elf, 0, hdr_size);
LOAD_HEADER;
shdr_size = sizeof(Elf64_Shdr);
}
@@ -417,7 +424,7 @@ int kmod_elf_get_section(const struct kmod_elf *elf, const char *section, const
if (!streq(section, n))
continue;
- *buf = elf_get_mem(elf, off);
+ *buf = elf_get_mem(elf, off, size);
*buf_size = size;
return 0;
}
@@ -534,7 +541,8 @@ int kmod_elf_get_modversions(const struct kmod_elf *elf, struct kmod_modversion
slen = 0;
for (i = 0; i < count; i++, off += MODVERSION_SEC_SIZE) {
- const char *symbol = elf_get_mem(elf, off + offcrc);
+ const char *symbol = elf_get_mem(elf, off + offcrc,
+ size - offcrc);
if (symbol[0] == '.')
symbol++;
@@ -551,7 +559,8 @@ int kmod_elf_get_modversions(const struct kmod_elf *elf, struct kmod_modversion
for (i = 0; i < count; i++, off += MODVERSION_SEC_SIZE) {
uint64_t crc = elf_get_uint(elf, off, offcrc);
- const char *symbol = elf_get_mem(elf, off + offcrc);
+ const char *symbol = elf_get_mem(elf, off + offcrc,
+ size - offcrc);
size_t symbollen;
if (symbol[0] == '.')
@@ -806,7 +815,8 @@ int kmod_elf_get_symbols(const struct kmod_elf *elf, struct kmod_modversion **ar
goto fallback;
}
- name = elf_get_mem(elf, str_off + name_off);
+ name = elf_get_mem(elf, str_off + name_off,
+ strtablen - name_off);
if (strncmp(name, crc_str, crc_strlen) != 0)
continue;
@@ -846,7 +856,8 @@ int kmod_elf_get_symbols(const struct kmod_elf *elf, struct kmod_modversion **ar
info = READV(st_info);
}
#undef READV
- name = elf_get_mem(elf, str_off + name_off);
+ name = elf_get_mem(elf, str_off + name_off,
+ strtablen - name_off);
if (strncmp(name, crc_str, crc_strlen) != 0)
continue;
name += crc_strlen;
@@ -889,7 +900,8 @@ static int kmod_elf_crc_find(const struct kmod_elf *elf, const void *versions, u
off = (const uint8_t *)versions - elf->memory;
for (i = 0; i < versionslen; i += verlen) {
- const char *symbol = elf_get_mem(elf, off + i + crclen);
+ const char *symbol = elf_get_mem(elf, off + i + crclen,
+ versionslen - i - crclen);
if (!streq(name, symbol))
continue;
*crc = elf_get_uint(elf, off + i, crclen);
@@ -1038,7 +1050,8 @@ int kmod_elf_get_dependency_symbols(const struct kmod_elf *elf, struct kmod_modv
return -EINVAL;
}
- name = elf_get_mem(elf, str_off + name_off);
+ name = elf_get_mem(elf, str_off + name_off,
+ strtablen - name_off);
if (name[0] == '\0') {
ELFDBG(elf, "empty symbol name at index %"PRIu64"\n", i);
continue;
@@ -1059,7 +1072,7 @@ int kmod_elf_get_dependency_symbols(const struct kmod_elf *elf, struct kmod_modv
for (i = 0; i < vercount; i++) {
if (visited_versions[i] == 0) {
const char *name;
- name = elf_get_mem(elf, ver_off + i * verlen + crclen);
+ name = elf_get_mem(elf, ver_off + i * verlen + crclen, verlen);
slen += strlen(name) + 1;
count++;
@@ -1126,7 +1139,8 @@ int kmod_elf_get_dependency_symbols(const struct kmod_elf *elf, struct kmod_modv
continue;
}
- name = elf_get_mem(elf, str_off + name_off);
+ name = elf_get_mem(elf, str_off + name_off,
+ strtablen - name_off);
if (name[0] == '\0') {
ELFDBG(elf, "empty symbol name at index %"PRIu64"\n", i);
continue;
@@ -1168,7 +1182,7 @@ int kmod_elf_get_dependency_symbols(const struct kmod_elf *elf, struct kmod_modv
if (visited_versions[i] != 0)
continue;
- name = elf_get_mem(elf, ver_off + i * verlen + crclen);
+ name = elf_get_mem(elf, ver_off + i * verlen + crclen, verlen);
slen = strlen(name);
crc = elf_get_uint(elf, ver_off + i * verlen, crclen);
--
2.3.0
reply other threads:[~2015-02-10 19:38 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150210193803.GA11559@localhost \
--to=tobias@stoeckmann.org \
--cc=linux-modules@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.