From: Ian Jackson <ian.jackson@eu.citrix.com>
To: xen-devel@lists.xensource.com
Cc: Stefano Stabellini <sstabellini@kernel.org>,
Wei Liu <wei.liu2@citrix.com>,
George Dunlap <George.Dunlap@eu.citrix.com>,
Andrew Cooper <andrew.cooper3@citrix.com>,
Tim Deegan <tim@xen.org>, Jan Beulich <JBeulich@suse.com>,
Ian Jackson <Ian.Jackson@eu.citrix.com>
Subject: [PATCH 3/8] libelf: loop safety: Call elf_iter_ok[_counted] in every loop
Date: Fri, 9 Dec 2016 15:44:44 +0000 [thread overview]
Message-ID: <1481298289-13546-4-git-send-email-ian.jackson@eu.citrix.com> (raw)
In-Reply-To: <1481298289-13546-1-git-send-email-ian.jackson@eu.citrix.com>
In every `for' or `while' loop, either call elf_iter_ok, or explain
why it's not necessary. This is part of comprehensive defence against
out of control loops.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
xen/common/libelf/libelf-dominfo.c | 22 +++++++++++++---------
xen/common/libelf/libelf-loader.c | 8 ++++----
xen/common/libelf/libelf-tools.c | 6 +++---
3 files changed, 20 insertions(+), 16 deletions(-)
diff --git a/xen/common/libelf/libelf-dominfo.c b/xen/common/libelf/libelf-dominfo.c
index 7f4a6a0..b139e32 100644
--- a/xen/common/libelf/libelf-dominfo.c
+++ b/xen/common/libelf/libelf-dominfo.c
@@ -43,10 +43,13 @@ elf_errorstatus elf_xen_parse_features(struct elf_binary *elf,
if ( features == NULL )
return 0;
- for ( pos = 0; features[pos] != '\0'; pos += len )
+ for ( pos = 0;
+ elf_iter_ok_counted(elf, sizeof(feature)) &&
+ features[pos] != '\0';
+ pos += len )
{
elf_memset_unchecked(feature, 0, sizeof(feature));
- for ( len = 0;; len++ )
+ for ( len = 0;; len++ ) /* can't do more than sizeof(feature) */
{
if ( len >= sizeof(feature)-1 )
break;
@@ -60,7 +63,7 @@ elf_errorstatus elf_xen_parse_features(struct elf_binary *elf,
feature[len] = features[pos + len];
}
- for ( i = 0; i < elf_xen_features; i++ )
+ for ( i = 0; elf_iter_ok(elf) && i < elf_xen_features; i++ )
{
if ( !elf_xen_feature_names[i] )
continue;
@@ -236,7 +239,7 @@ static unsigned elf_xen_parse_notes(struct elf_binary *elf,
parms->elf_note_start = start;
parms->elf_note_end = end;
for ( note = ELF_MAKE_HANDLE(elf_note, parms->elf_note_start);
- ELF_HANDLE_PTRVAL(note) < parms->elf_note_end;
+ elf_iter_ok(elf) && ELF_HANDLE_PTRVAL(note) < parms->elf_note_end;
note = elf_note_next(elf, note) )
{
#ifdef __XEN__
@@ -273,11 +276,12 @@ elf_errorstatus elf_xen_parse_guest_info(struct elf_binary *elf,
h = parms->guest_info;
#define STAR(h) (elf_access_unsigned(elf, (h), 0, 1))
- while ( STAR(h) )
+ while ( elf_iter_ok_counted(elf, sizeof(name) + sizeof(value)) &&
+ STAR(h) )
{
elf_memset_unchecked(name, 0, sizeof(name));
elf_memset_unchecked(value, 0, sizeof(value));
- for ( len = 0;; len++, h++ )
+ for ( len = 0;; len++, h++ ) /* covered by iter_ok_counted above */
{
if ( len >= sizeof(name)-1 )
break;
@@ -291,7 +295,7 @@ elf_errorstatus elf_xen_parse_guest_info(struct elf_binary *elf,
if ( STAR(h) == '=' )
{
h++;
- for ( len = 0;; len++, h++ )
+ for ( len = 0;; len++, h++ ) /* covered by iter_ok_counted */
{
if ( len >= sizeof(value)-1 )
break;
@@ -504,7 +508,7 @@ elf_errorstatus elf_xen_parse(struct elf_binary *elf,
/* Find and parse elf notes. */
count = elf_phdr_count(elf);
- for ( i = 0; i < count; i++ )
+ for ( i = 0; elf_iter_ok(elf) && i < count; i++ )
{
phdr = elf_phdr_by_index(elf, i);
if ( !elf_access_ok(elf, ELF_HANDLE_PTRVAL(phdr), 1) )
@@ -537,7 +541,7 @@ elf_errorstatus elf_xen_parse(struct elf_binary *elf,
if ( xen_elfnotes == 0 )
{
count = elf_shdr_count(elf);
- for ( i = 1; i < count; i++ )
+ for ( i = 1; elf_iter_ok(elf) && i < count; i++ )
{
shdr = elf_shdr_by_index(elf, i);
if ( !elf_access_ok(elf, ELF_HANDLE_PTRVAL(shdr), 1) )
diff --git a/xen/common/libelf/libelf-loader.c b/xen/common/libelf/libelf-loader.c
index 00479af..68c9021 100644
--- a/xen/common/libelf/libelf-loader.c
+++ b/xen/common/libelf/libelf-loader.c
@@ -85,7 +85,7 @@ elf_errorstatus elf_init(struct elf_binary *elf, const char *image_input, size_t
/* Find symbol table and symbol string table. */
count = elf_shdr_count(elf);
- for ( i = 1; i < count; i++ )
+ for ( i = 1; elf_iter_ok(elf) && i < count; i++ )
{
shdr = elf_shdr_by_index(elf, i);
if ( !elf_access_ok(elf, ELF_HANDLE_PTRVAL(shdr), 1) )
@@ -425,7 +425,7 @@ do { \
* NB: this _must_ be done one by one, and taking the bitness into account,
* so that the guest can treat this as an array of type Elf{32/64}_Shdr.
*/
- for ( i = 0; i < ELF_BSDSYM_SECTIONS; i++ )
+ for ( i = 0; elf_iter_ok(elf) && i < ELF_BSDSYM_SECTIONS; i++ )
{
rc = elf_load_image(elf, header_base + header_size + shdr_size * i,
ELF_REALPTR2PTRVAL(&header.elf_header.section[i]),
@@ -453,7 +453,7 @@ void elf_parse_binary(struct elf_binary *elf)
unsigned i, count;
count = elf_phdr_count(elf);
- for ( i = 0; i < count; i++ )
+ for ( i = 0; elf_iter_ok(elf) && i < count; i++ )
{
phdr = elf_phdr_by_index(elf, i);
if ( !elf_access_ok(elf, ELF_HANDLE_PTRVAL(phdr), 1) )
@@ -490,7 +490,7 @@ elf_errorstatus elf_load_binary(struct elf_binary *elf)
uint64_t remain_allow_copy = (uint64_t)elf->dest_size * 2;
count = elf_phdr_count(elf);
- for ( i = 0; i < count; i++ )
+ for ( i = 0; elf_iter_ok(elf) && i < count; i++ )
{
phdr = elf_phdr_by_index(elf, i);
if ( !elf_access_ok(elf, ELF_HANDLE_PTRVAL(phdr), 1) )
diff --git a/xen/common/libelf/libelf-tools.c b/xen/common/libelf/libelf-tools.c
index a9edb6a..56dab63 100644
--- a/xen/common/libelf/libelf-tools.c
+++ b/xen/common/libelf/libelf-tools.c
@@ -153,7 +153,7 @@ ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_name(struct elf_binary *elf, const char *n
ELF_HANDLE_DECL(elf_shdr) shdr;
const char *sname;
- for ( i = 1; i < count; i++ )
+ for ( i = 1; elf_iter_ok(elf) && i < count; i++ )
{
shdr = elf_shdr_by_index(elf, i);
if ( !elf_access_ok(elf, ELF_HANDLE_PTRVAL(shdr), 1) )
@@ -214,7 +214,7 @@ const char *elf_strval(struct elf_binary *elf, elf_ptrval start)
if ( !elf_access_unsigned(elf, start, length, 1) )
/* ok */
return ELF_UNSAFE_PTR(start);
- if ( length >= ELF_MAX_STRING_LENGTH )
+ if ( !elf_iter_ok(elf) || length >= ELF_MAX_STRING_LENGTH )
{
elf_mark_broken(elf, "excessively long string");
return NULL;
@@ -262,7 +262,7 @@ ELF_HANDLE_DECL(elf_sym) elf_sym_by_name(struct elf_binary *elf, const char *sym
uint64_t info, name;
const char *sym_name;
- for ( ; ptr < end; ptr += elf_size(elf, sym) )
+ for ( ; elf_iter_ok(elf) && ptr < end; ptr += elf_size(elf, sym) )
{
sym = ELF_MAKE_HANDLE(elf_sym, ptr);
info = elf_uval(elf, sym, st_info);
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
next prev parent reply other threads:[~2016-12-09 15:44 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-08 14:18 [PATCH] libelf: Fix div0 issues in elf_{shdr, phdr}_count() Andrew Cooper
2016-12-08 14:41 ` Jan Beulich
2016-12-08 14:46 ` Andrew Cooper
2016-12-08 15:17 ` Jan Beulich
2016-12-08 15:23 ` Andrew Cooper
2016-12-08 15:47 ` Ian Jackson
2016-12-08 16:09 ` Jan Beulich
2016-12-08 17:28 ` Ian Jackson
2016-12-09 8:38 ` Jan Beulich
2016-12-09 11:54 ` Ian Jackson
2016-12-09 13:03 ` Jan Beulich
2016-12-09 15:44 ` [PATCH 0/8] libelf: safety enhancements Ian Jackson
2016-12-09 15:44 ` [PATCH 1/8] libelf: loop safety: Introduce elf_iter_ok and elf_strcmp_safe Ian Jackson
2016-12-12 15:02 ` Jan Beulich
2016-12-12 15:23 ` Ian Jackson
2016-12-12 15:15 ` Jan Beulich
2016-12-12 15:51 ` Jan Beulich
2016-12-12 16:00 ` Ian Jackson
2016-12-12 16:16 ` Jan Beulich
2016-12-12 16:56 ` Ian Jackson
2016-12-13 7:24 ` Jan Beulich
2016-12-13 16:04 ` Ian Jackson
2016-12-13 16:37 ` Jan Beulich
2016-12-09 15:44 ` [PATCH 2/8] libelf: loop safety: Pass `elf' to elf_xen_parse_features Ian Jackson
2016-12-12 15:03 ` Jan Beulich
2016-12-09 15:44 ` Ian Jackson [this message]
2016-12-12 15:12 ` [PATCH 3/8] libelf: loop safety: Call elf_iter_ok[_counted] in every loop Jan Beulich
2016-12-12 15:38 ` Ian Jackson
2016-12-12 15:56 ` Jan Beulich
2016-12-12 16:02 ` Ian Jackson
2016-12-09 15:44 ` [PATCH 4/8] libelf: loop safety: Call elf_iter_ok_counted at every *mem*_unsafe Ian Jackson
2016-12-12 15:19 ` Jan Beulich
2016-12-12 15:54 ` Ian Jackson
2016-12-12 15:58 ` Jan Beulich
2016-12-12 16:03 ` Ian Jackson
2016-12-09 15:44 ` [PATCH 5/8] libelf: loop safety: Replace all calls to strcmp Ian Jackson
2016-12-12 15:22 ` Jan Beulich
2016-12-12 15:44 ` Ian Jackson
2016-12-09 15:44 ` [PATCH 6/8] libelf: loop safety cleanup: Remove obsolete check in elf_shdr_count Ian Jackson
2016-12-12 15:41 ` Jan Beulich
2016-12-09 15:44 ` [PATCH 7/8] libelf: loop safety cleanup: Remove superseded image size copy check Ian Jackson
2016-12-12 16:26 ` Jan Beulich
2016-12-09 15:44 ` [PATCH 8/8] libelf: safety: Document safety principles in header file Ian Jackson
2016-12-15 16:43 ` Jan Beulich
2016-12-16 4:28 ` George Dunlap
2016-12-16 11:33 ` Ian Jackson
2016-12-16 11:58 ` Jan Beulich
2016-12-16 11:43 ` Ian Jackson
2016-12-16 12:31 ` Jan Beulich
2016-12-08 14:48 ` [PATCH] libelf: Fix div0 issues in elf_{shdr, phdr}_count() Ian Jackson
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=1481298289-13546-4-git-send-email-ian.jackson@eu.citrix.com \
--to=ian.jackson@eu.citrix.com \
--cc=George.Dunlap@eu.citrix.com \
--cc=JBeulich@suse.com \
--cc=andrew.cooper3@citrix.com \
--cc=sstabellini@kernel.org \
--cc=tim@xen.org \
--cc=wei.liu2@citrix.com \
--cc=xen-devel@lists.xensource.com \
/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 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 NNTP newsgroup(s).