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