On 8/21/20 7:02 PM, Kristen Carlson Accardi wrote:
On Wed, 2020-07-22 at 16:33 -0500, Josh Poimboeuf wrote:
On Wed, Jul 22, 2020 at 12:56:10PM -0700, Kristen Carlson Accardi
wrote:
On Wed, 2020-07-22 at 12:42 -0700, Kees Cook wrote:
On Wed, Jul 22, 2020 at 11:07:30AM -0500, Josh Poimboeuf wrote:
On Wed, Jul 22, 2020 at 07:39:55AM -0700, Kees Cook wrote:
On Wed, Jul 22, 2020 at 11:27:30AM +0200, Miroslav Benes
wrote:
Let me CC live-patching ML, because from a quick glance
this is
something
which could impact live patching code. At least it
invalidates
assumptions
which "sympos" is based on.
In a quick skim, it looks like the symbol resolution is using
kallsyms_on_each_symbol(), so I think this is safe? What's a
good
selftest for live-patching?
The problem is duplicate symbols. If there are two static
functions
named 'foo' then livepatch needs a way to distinguish them.
Our current approach to that problem is "sympos". We rely on
the
fact
that the second foo() always comes after the first one in the
symbol
list and kallsyms. So they're referred to as foo,1 and foo,2.
Ah. Fun. In that case, perhaps the LTO series has some solutions.
I
think builds with LTO end up renaming duplicate symbols like
that, so
it'll be back to being unique.
Well, glad to hear there might be some precendence for how to solve
this, as I wasn't able to think of something reasonable off the top
of
my head. Are you speaking of the Clang LTO series?
https://lore.kernel.org/lkml/20200624203200.78870-1-samitolvanen@xxxxxxxxxx/
I'm not sure how LTO does it, but a few more (half-brained) ideas
that
could work:
1) Add a field in kallsyms to keep track of a symbol's original
offset
before randomization/re-sorting. Livepatch could use that field
to
determine the original sympos.
2) In fgkaslr code, go through all the sections and mark the ones
which
have duplicates (i.e. same name). Then when shuffling the
sections,
skip a shuffle if it involves a duplicate section. That way all
the
duplicates would retain their original sympos.
3) Livepatch could uniquely identify symbols by some feature other
than
sympos. For example:
Symbol/function size - obviously this would only work if
duplicately
named symbols have different sizes.
Checksum - as part of a separate feature we're also looking at
giving
each function its own checksum, calculated based on its
instruction
opcodes. Though calculating checksums at runtime could be
complicated by IP-relative addressing.
I'm thinking #1 or #2 wouldn't be too bad. #3 might be harder.
Hi there! I was trying to find a super easy way to address this, so I
thought the best thing would be if there were a compiler or linker
switch to just eliminate any duplicate symbols at compile time for
vmlinux. I filed this question on the binutils bugzilla looking to see
if there were existing flags that might do this, but H.J. Lu went ahead
and created a new one "-z unique", that seems to do what we would need
it to do.
https://sourceware.org/bugzilla/show_bug.cgi?id=26391
When I use this option, it renames any duplicate symbols with an
extension - for example duplicatefunc.1 or duplicatefunc.2.
I tried out H.J. Lu's branch and built some of the livepatch selftests
with -z unique-symbol and indeed observe the following pattern:
foo, foo.1, foo.2, etc.
for homonym symbol names.
> You could
either match on the full unique name of the specific binary you are
trying to patch, or you match the base name and use the extension to
determine original position. Do you think this solution would work?
I think it could work for klp-relocations.
As a quick test, I was able to hack the WIP klp-convert branch [1] to
generate klp-relocations with the following hack:
const char *foo(void) __asm__("foo.1");
when building foo's target with -z unique-symbol. (The target contained
two static foo() functions.) The asm rename trick exercised the
klp-convert implicit conversion feature, as the symbol was now uniquely
named and included a non-valid C symbol character. User-defined
klp-convert annotation support will require some refactoring, but
shouldn't be too difficult to support as well.
If
so, I can modify livepatch to refuse to patch on duplicated symbols if
CONFIG_FG_KASLR and when this option is merged into the tool chain I
can add it to KBUILD_LDFLAGS when CONFIG_FG_KASLR and livepatching
should work in all cases.
I don't have a grasp on how complicated the alternatives might be, so
I'll let others comment on best paths forward. I just wanted to note
that -z unique-symbol looks like it could reasonable work well for this
niche case.
[1]
https://github.com/joe-lawrence/linux/tree/klp-convert-v5-expanded-v5.8
(not modified for -z unique-symbol, but noted for reference)
-- Joe
