asan: mmo: NULL dereferenc in mmo_xore_32
Hans-Peter Nilsson
hp@bitrange.com
Thu Oct 28 22:38:48 GMT 2021
On Thu, 28 Oct 2021, Alan Modra via Binutils wrote:
> mmo_get_loc can return NULL. It's commented even, and that the caller
> then must handle a split field. mmo_xore_* don't handle split fields,
Thanks for fixing this. I *think* I meant that the requests
from mmo_xore_<N>, N=(16|32|64) would be covered by the second
clause at the comment in question; "Requests with an address
aligned on MMO_SEC_CONTENTS_CHUNK_SIZE bytes and for no more
than MMO_SEC_CONTENTS_CHUNK_SIZE will always get resolved."
(MMO_SEC_CONTENTS_CHUNK_SIZE=32768) but I guess that fails for
example if some caller is tricked into sneaking in a
non-N-aligned vma.
> instead just segfault. Stop that happening, and refuse to recognise
> fuzzed mmo files that trigger this problem.
It would be nice to have test-cases covering those new code
paths; I think I had 100% coverage at one time. Is there a
fuzzed object located somewhere that I can look at?
I'm putting it on my todo-list, together with changing
mmo_xore_<N> to return bool so no-one (like me) gets confused by
the only uses of boolean use of the return-value.
>
> * mmo.c (mmo_get_loc): Don't declare inline.
Yeah, it got a little bit too bit for that... IMHO inline on
static functions is a little bit presumptious these days.
I'll just drop them altogether.
> (mmo_xore_64, mmo_xore_32, mmo_xore_16): Remove forward decls.
> Return pointer, don't dereference NULL.
> (mmo_scan): Return error on mmo_get_loc returning NULL.
brgds, H-P
More information about the Binutils
mailing list