All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v7 00/22] XSA55 libelf fixes for unstable
@ 2013-06-11 18:20 Ian Jackson
  2013-06-11 18:20 ` [PATCH 01/22] libelf: abolish libelf-relocate.c Ian Jackson
                   ` (22 more replies)
  0 siblings, 23 replies; 51+ messages in thread
From: Ian Jackson @ 2013-06-11 18:20 UTC (permalink / raw)
  To: xen-devel; +Cc: andrew.cooper3, mattjd, security

This is version 7 of my series to try to fix libelf and the domain
loader.

This is available via git:
  http://xenbits.xen.org/gitweb/?p=people/iwj/xen-unstable.git;a=summary
  git://xenbits.xen.org/people/iwj/xen-unstable.git
in the commits
  xsa55-unstable-base-rebasing..xsa55-unstable-rebasing

Here is a summary of the state of series:

A  01/21 libelf: abolish libelf-relocate.c
   02/21 libxc: introduce xc_dom_seg_to_ptr_pages
   03/21 libxc: Fix range checking in xc_dom_pfn_to_ptr etc.
A  04/21 libelf: add `struct elf_binary*' parameter to elf_load_image
A  05/21 libelf: abolish elf_sval and elf_access_signed
A  06/21 libelf: move include of <asm/guest_access.h> to top of file
A  07/21 libelf/xc_dom_load_elf_symtab: Do not use "syms" uninitialised
A* 08/21 libxl: introduce macros for memory access and pointer handling
A  09/21 tools/xcutils/readnotes: adjust print_l1_mfn_valid_note
A- 10/21 libelf: check nul-terminated strings properly
 - 11/21 libxl: check all pointer accesses
A- 12/21 libxl: Check pointer references in elf_is_elfbinary
A  13/21 libelf: Make all callers call elf_check_broken
a  14/21 libelf: use C99 bool for booleans
   15/21 libelf: use only unsigned integers
   16/21 libelf: check loops for running away
A  17/21 libelf: abolish obsolete macros
   18/21 libxc: Add range checking to xc_dom_binloader
 * 19/21 libxc: check failure of xc_dom_*_to_ptr, xc_map_foreign_range
 * 20/21 libxc: check return values from malloc
   21/21 libxc: range checks in xc_dom_p2m_host and _guest
   22/22 libxc: check blob size before proceeding in xc_dom_check_gzip

Key to symbols:
 *   Updated in this version of the series.
 +   New patch in this version.
 /   Updated but only to remove changes into a separate patch.
 -   Updated with style, comment or commit message changes only.
 a   Acked/reviwed by one reviewer.
 A   Acked/reviwed by more than one reviewer.
Also in every patch:
Updated commit msgs to correct email address for me.


libelf, and some of its callers, did not do nearly enough checking on
their input.  Invalid inputs could cause signed integer arithmetic
overflows and wild pointer dereferences.

In this series we try to systematically eliminate this problem in a
way which has a reasonable chance of (i) still accepting all
previously-accepted ELF images (ii) not having remaining security
bugs, in a form which can be reviewied to verify (i) and (ii).

Additionally, we fix some related security bugs in the domain loading
code.

The approach is:

(i) Remove all uses of signed integers (of any kind).  That
    elmininates all integer overflows as sources of undefined
    behaviour.  Of course it still means that we can get incorrect
    values throughout the code.

(ii) Replace all uses of pointers, both pointers into the supplied
    ELF image, and pointers into the output (where we are loading)
    by uintptr_t.  That eliminates all pointer arithmetic overflows as
    sources of undefined behaviour.  Of course it still means that we
    can get incorrect and unreasonable "pointer" values.

(iii) But these pointer values will be in uintptr_t, which cannot be
    simply dereferenced by mistake.  We will replace all dereferences
    by macros which do range checking; out of range reads will read 0
    and out of range writes will be ignored.  Happily most (but not
    all) of the reads in the code already go through macros which
    abstract endianness[1] and/or 32/64bitness.

    [1] Although not all the accesses use endian-aware techniques so
    in fact the code can't cope with foreign-endian ELFs.  This is a
    problem for another day.

(iv) Look for all loops and check that they are guaranteed to
    terminate.

To enable verification of correctness of these changes I provide them
as a series roughly as follows:

1-6:
   Pre-patches which make a few semantically neutral or semantically
   correct changes.  For human review.

7: Introduces a set of macros to abstract away pointer arithmetic and
   input and output image memory accesses in libelf.  Use these macros
   everwhere they are applicable.  However, define the macros in a way
   that corresponds to the existing code.  That this patch has no
   functional change can be verified by comparing the before-and-after
   assembler output from the compiler.

9. Introduce some macros for dealing with nul-terminated strings,
   defined so as not to have any functional change at this stage.

10. Change the macro definitions, and introduce the new pseudopointer
   types, pointer range checking, etc.  For close human review.  Each
   macro change is justified in the commit message.  This patch
   eliminates most of the potential wild pointer accesses.

8,11-12,14-15:
   Smaller patches for human review, fixing some leftover bugs,
   including ensuring that all loops terminate.

13: Eliminate signed integers.  Replace every "int", "int*_t",
   "long" and most "char"s by corresponding unsigned types.  This
    eliminates all integer arithmetic overflows.

After this patch, libelf should be safe against hostile input:

 * All arithmetic operations on values from the input file use
   unsigned arithmetic which is guaranteed to be defined (although
   it may of course result in wrong answers);

 * All pointer accesses based on pointers to locations which depend on
   the input file go via our range-checking accessors; accesses which
   are not to the input or output regions are ignored (reads returning
   0).

 * The loops have been checked to ensure that they terminate and are at
   worst O(image_size).

 * Whenever an array variable was declared, the code has been manually
   reviewed looking for possible out-of-bounds accesses.

This is XSA-55.

^ permalink raw reply	[flat|nested] 51+ messages in thread
* [PATCH v7 00/22] XSA55 libelf fixes for Xen 4.2
@ 2013-06-11 18:22 Ian Jackson
  2013-06-11 18:22 ` [PATCH 20/22] libxc: check return values from malloc Ian Jackson
  0 siblings, 1 reply; 51+ messages in thread
From: Ian Jackson @ 2013-06-11 18:22 UTC (permalink / raw)
  To: xen-devel; +Cc: andrew.cooper3, mattjd, security

This is a backport of my series to try to fix libelf and the domain
loader.  It corresponds to v7 of the xen-unstable series.

This is available via git:
  http://xenbits.xen.org/gitweb/?p=people/iwj/xen-unstable.git;a=summary
  git://xenbits.xen.org/people/iwj/xen-unstable.git
in the commits
  xsa55-4.2-base-rebasing..xsa55-4.2-rebasing

Here is a summary of the series:

   01/21 libelf: abolish libelf-relocate.c
   02/21 libxc: introduce xc_dom_seg_to_ptr_pages
   03/21 libxc: Fix range checking in xc_dom_pfn_to_ptr etc.
   04/21 libelf: add `struct elf_binary*' parameter to elf_load_image
   05/21 libelf: abolish elf_sval and elf_access_signed
   06/21 libelf: move include of <asm/guest_access.h> to top of file
   07/21 libelf/xc_dom_load_elf_symtab: Do not use "syms" uninitialised
   08/21 libxl: introduce macros for memory access and pointer handling
   09/21 tools/xcutils/readnotes: adjust print_l1_mfn_valid_note
   10/21 libelf: check nul-terminated strings properly
   11/21 libxl: check all pointer accesses
   12/21 libxl: Check pointer references in elf_is_elfbinary
   13/21 libelf: Make all callers call elf_check_broken
   14/21 libelf: use C99 bool for booleans
   15/21 libelf: use only unsigned integers
   16/21 libelf: check loops for running away
   17/21 libelf: abolish obsolete macros
   18/21 libxc: Add range checking to xc_dom_binloader
   19/21 libxc: check failure of xc_dom_*_to_ptr, xc_map_foreign_range
   20/21 libxc: check return values from malloc
   21/21 libxc: range checks in xc_dom_p2m_host and _guest
   22/22 libxc: check blob size before proceeding in xc_dom_check_gzip

Please refer to the 00 -unstable message for more information.

^ permalink raw reply	[flat|nested] 51+ messages in thread
* [PATCH v6 00/22] XSA55 libelf fixes for Xen 4.2
@ 2013-06-07 18:35 Ian Jackson
  2013-06-07 18:35 ` [PATCH 20/22] libxc: check return values from malloc Ian Jackson
  0 siblings, 1 reply; 51+ messages in thread
From: Ian Jackson @ 2013-06-07 18:35 UTC (permalink / raw)
  To: xen-devel; +Cc: andrew.cooper3, mattjd, security

This is a backport of my series to try to fix libelf and the domain
loader.  It corresponds to v6 of the xen-unstable series.

This is available via git:
  http://xenbits.xen.org/gitweb/?p=people/iwj/xen-unstable.git;a=summary
  git://xenbits.xen.org/people/iwj/xen-unstable.git
in the commits
  xsa55-4.2-base-rebasing..xsa55-4.2-rebasing

Here is a summary of the state of series:

a  01/21 libelf: abolish libelf-relocate.c
   02/21 libxc: introduce xc_dom_seg_to_ptr_pages
   03/21 libxc: Fix range checking in xc_dom_pfn_to_ptr etc.
A  04/21 libelf: add `struct elf_binary*' parameter to elf_load_image
a  05/21 libelf: abolish elf_sval and elf_access_signed
A  06/21 libelf: move include of <asm/guest_access.h> to top of file
a  07/21 libelf/xc_dom_load_elf_symtab: Do not use "syms" uninitialised
   08/21 libxl: introduce macros for memory access and pointer handling
a  09/21 tools/xcutils/readnotes: adjust print_l1_mfn_valid_note
A  10/21 libelf: check nul-terminated strings properly
   11/21 libxl: check all pointer accesses
A  12/21 libxl: Check pointer references in elf_is_elfbinary
a# 13/21 libelf: Make all callers call elf_check_broken
a  14/21 libelf: use C99 bool for booleans
   15/21 libelf: use only unsigned integers
   16/21 libelf: check loops for running away
a  17/21 libelf: abolish obsolete macros
   18/21 libxc: Add range checking to xc_dom_binloader
 # 19/21 libxc: check failure of xc_dom_*_to_ptr, xc_map_foreign_range
 # 20/21 libxc: check return values from malloc
 # 21/21 libxc: range checks in xc_dom_p2m_host and _guest
   22/22 libxc: check blob size before proceeding in xc_dom_check_gzip

Key to symbols:
 a   Acked/reviwed by one reviewer.           } Refers to acks of
 A   Acked/reviwed by more than one reviewer. }  -unstable series.
 #   Nontrivial differences between series for -unstable and 4.2.

Please refer to the v6 00/22 -unstable message for more information.

^ permalink raw reply	[flat|nested] 51+ messages in thread
* [PATCH v6 00/22] XSA55 libelf fixes for unstable
@ 2013-06-07 18:27 Ian Jackson
  2013-06-07 18:27 ` [PATCH 20/22] libxc: check return values from malloc Ian Jackson
  0 siblings, 1 reply; 51+ messages in thread
From: Ian Jackson @ 2013-06-07 18:27 UTC (permalink / raw)
  To: xen-devel; +Cc: andrew.cooper3, mattjd, security

This is version 6 of my series to try to fix libelf and the domain
loader.

This is available via git:
  http://xenbits.xen.org/gitweb/?p=people/iwj/xen-unstable.git;a=summary
  git://xenbits.xen.org/people/iwj/xen-unstable.git
in the commits
  xsa55-unstable-base-rebasing..xsa55-unstable-rebasing

Here is a summary of the state of series:

a  01/21 libelf: abolish libelf-relocate.c
   02/21 libxc: introduce xc_dom_seg_to_ptr_pages
   03/21 libxc: Fix range checking in xc_dom_pfn_to_ptr etc.
A  04/21 libelf: add `struct elf_binary*' parameter to elf_load_image
a  05/21 libelf: abolish elf_sval and elf_access_signed
A  06/21 libelf: move include of <asm/guest_access.h> to top of file
a  07/21 libelf/xc_dom_load_elf_symtab: Do not use "syms" uninitialised
   08/21 libxl: introduce macros for memory access and pointer handling
a  09/21 tools/xcutils/readnotes: adjust print_l1_mfn_valid_note
A  10/21 libelf: check nul-terminated strings properly
   11/21 libxl: check all pointer accesses
A  12/21 libxl: Check pointer references in elf_is_elfbinary
a  13/21 libelf: Make all callers call elf_check_broken
a  14/21 libelf: use C99 bool for booleans
   15/21 libelf: use only unsigned integers
   16/21 libelf: check loops for running away
a  17/21 libelf: abolish obsolete macros
 * 18/21 libxc: Add range checking to xc_dom_binloader
   19/21 libxc: check failure of xc_dom_*_to_ptr, xc_map_foreign_range
 * 20/21 libxc: check return values from malloc
 * 21/21 libxc: range checks in xc_dom_p2m_host and _guest
 * 22/22 libxc: check blob size before proceeding in xc_dom_check_gzip

Key to symbols:
 *   Updated in this version of the series.
 +   New patch in this version.
 /   Updated but only to remove changes into a separate patch.
 -   Updated with style changes only.
 a   Acked/reviwed by one reviewer.
 A   Acked/reviwed by more than one reviewer.
Also in every patch:
Updated commit msgs to correct email address for me.


libelf, and some of its callers, did not do nearly enough checking on
their input.  Invalid inputs could cause signed integer arithmetic
overflows and wild pointer dereferences.

In this series we try to systematically eliminate this problem in a
way which has a reasonable chance of (i) still accepting all
previously-accepted ELF images (ii) not having remaining security
bugs, in a form which can be reviewied to verify (i) and (ii).

Additionally, we fix some related security bugs in the domain loading
code.

The approach is:

(i) Remove all uses of signed integers (of any kind).  That
    elmininates all integer overflows as sources of undefined
    behaviour.  Of course it still means that we can get incorrect
    values throughout the code.

(ii) Replace all uses of pointers, both pointers into the supplied
    ELF image, and pointers into the output (where we are loading)
    by uintptr_t.  That eliminates all pointer arithmetic overflows as
    sources of undefined behaviour.  Of course it still means that we
    can get incorrect and unreasonable "pointer" values.

(iii) But these pointer values will be in uintptr_t, which cannot be
    simply dereferenced by mistake.  We will replace all dereferences
    by macros which do range checking; out of range reads will read 0
    and out of range writes will be ignored.  Happily most (but not
    all) of the reads in the code already go through macros which
    abstract endianness[1] and/or 32/64bitness.

    [1] Although not all the accesses use endian-aware techniques so
    in fact the code can't cope with foreign-endian ELFs.  This is a
    problem for another day.

(iv) Look for all loops and check that they are guaranteed to
    terminate.

To enable verification of correctness of these changes I provide them
as a series roughly as follows:

1-6:
   Pre-patches which make a few semantically neutral or semantically
   correct changes.  For human review.

7: Introduces a set of macros to abstract away pointer arithmetic and
   input and output image memory accesses in libelf.  Use these macros
   everwhere they are applicable.  However, define the macros in a way
   that corresponds to the existing code.  That this patch has no
   functional change can be verified by comparing the before-and-after
   assembler output from the compiler.

9. Introduce some macros for dealing with nul-terminated strings,
   defined so as not to have any functional change at this stage.

10. Change the macro definitions, and introduce the new pseudopointer
   types, pointer range checking, etc.  For close human review.  Each
   macro change is justified in the commit message.  This patch
   eliminates most of the potential wild pointer accesses.

8,11-12,14-15:
   Smaller patches for human review, fixing some leftover bugs,
   including ensuring that all loops terminate.

13: Eliminate signed integers.  Replace every "int", "int*_t",
   "long" and most "char"s by corresponding unsigned types.  This
    eliminates all integer arithmetic overflows.

After this patch, libelf should be safe against hostile input:

 * All arithmetic operations on values from the input file use
   unsigned arithmetic which is guaranteed to be defined (although
   it may of course result in wrong answers);

 * All pointer accesses based on pointers to locations which depend on
   the input file go via our range-checking accessors; accesses which
   are not to the input or output regions are ignored (reads returning
   0).

 * The loops have been checked to ensure that they terminate and are at
   worst O(image_size).

 * Whenever an array variable was declared, the code has been manually
   reviewed looking for possible out-of-bounds accesses.

This is XSA-55.

^ permalink raw reply	[flat|nested] 51+ messages in thread

end of thread, other threads:[~2013-06-12 18:26 UTC | newest]

Thread overview: 51+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-11 18:20 [PATCH v7 00/22] XSA55 libelf fixes for unstable Ian Jackson
2013-06-11 18:20 ` [PATCH 01/22] libelf: abolish libelf-relocate.c Ian Jackson
2013-06-11 18:20 ` [PATCH 02/22] libxc: introduce xc_dom_seg_to_ptr_pages Ian Jackson
2013-06-11 18:44   ` Andrew Cooper
2013-06-11 18:20 ` [PATCH 03/22] libxc: Fix range checking in xc_dom_pfn_to_ptr etc Ian Jackson
2013-06-11 19:01   ` Andrew Cooper
2013-06-11 18:20 ` [PATCH 04/22] libelf: add `struct elf_binary*' parameter to elf_load_image Ian Jackson
2013-06-11 18:20 ` [PATCH 05/22] libelf: abolish elf_sval and elf_access_signed Ian Jackson
2013-06-11 18:20 ` [PATCH 06/22] libelf: move include of <asm/guest_access.h> to top of file Ian Jackson
2013-06-11 18:20 ` [PATCH 07/22] libelf/xc_dom_load_elf_symtab: Do not use "syms" uninitialised Ian Jackson
2013-06-11 18:20 ` [PATCH 08/22] libelf: introduce macros for memory access and pointer handling Ian Jackson
2013-06-11 18:20 ` [PATCH 09/22] tools/xcutils/readnotes: adjust print_l1_mfn_valid_note Ian Jackson
2013-06-11 18:20 ` [PATCH 10/22] libelf: check nul-terminated strings properly Ian Jackson
2013-06-11 19:14   ` Andrew Cooper
2013-06-11 18:20 ` [PATCH 11/22] libelf: check all pointer accesses Ian Jackson
2013-06-11 19:19   ` Andrew Cooper
2013-06-11 18:20 ` [PATCH 12/22] libelf: Check pointer references in elf_is_elfbinary Ian Jackson
2013-06-11 19:03   ` Andrew Cooper
2013-06-11 18:20 ` [PATCH 13/22] libelf: Make all callers call elf_check_broken Ian Jackson
2013-06-11 18:20 ` [PATCH 14/22] libelf: use C99 bool for booleans Ian Jackson
2013-06-11 19:04   ` Andrew Cooper
2013-06-11 18:20 ` [PATCH 15/22] libelf: use only unsigned integers Ian Jackson
2013-06-11 19:22   ` Andrew Cooper
2013-06-11 18:20 ` [PATCH 16/22] libelf: check loops for running away Ian Jackson
2013-06-11 19:28   ` Andrew Cooper
2013-06-11 18:21 ` [PATCH 17/22] libelf: abolish obsolete macros Ian Jackson
2013-06-11 18:21 ` [PATCH 18/22] libxc: Add range checking to xc_dom_binloader Ian Jackson
2013-06-11 19:11   ` Andrew Cooper
2013-06-11 18:21 ` [PATCH 19/22] libxc: check failure of xc_dom_*_to_ptr, xc_map_foreign_range Ian Jackson
2013-06-11 19:10   ` Andrew Cooper
2013-06-11 18:21 ` [PATCH 20/22] libxc: check return values from malloc Ian Jackson
2013-06-11 19:05   ` Andrew Cooper
2013-06-11 18:21 ` [PATCH 21/22] libxc: range checks in xc_dom_p2m_host and _guest Ian Jackson
2013-06-11 19:06   ` Andrew Cooper
2013-06-12 16:02   ` George Dunlap
2013-06-12 16:06     ` Ian Jackson
2013-06-12 16:08       ` George Dunlap
2013-06-12 18:26       ` Tim Deegan
2013-06-11 18:21 ` [PATCH 22/22] libxc: check blob size before proceeding in xc_dom_check_gzip Ian Jackson
2013-06-11 19:08   ` Andrew Cooper
2013-06-11 19:30 ` [PATCH v7 00/22] XSA55 libelf fixes for unstable Andrew Cooper
2013-06-12 13:45   ` Ian Jackson
2013-06-12 14:02     ` Ian Jackson
2013-06-12 14:19     ` Andrew Cooper
  -- strict thread matches above, loose matches on Subject: below --
2013-06-11 18:22 [PATCH v7 00/22] XSA55 libelf fixes for Xen 4.2 Ian Jackson
2013-06-11 18:22 ` [PATCH 20/22] libxc: check return values from malloc Ian Jackson
2013-06-07 18:35 [PATCH v6 00/22] XSA55 libelf fixes for Xen 4.2 Ian Jackson
2013-06-07 18:35 ` [PATCH 20/22] libxc: check return values from malloc Ian Jackson
2013-06-07 18:27 [PATCH v6 00/22] XSA55 libelf fixes for unstable Ian Jackson
2013-06-07 18:27 ` [PATCH 20/22] libxc: check return values from malloc Ian Jackson
2013-06-07 18:32   ` Andrew Cooper
2013-06-11 15:53     ` Ian Jackson
2013-06-10 20:38   ` Andrew Cooper
2013-06-11 14:31     ` Ian Jackson

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.