Subject: bug#58334: 29.0.50; ASAN heap use after free in gui_produce_glyphs

From: Eli Zaretskii <[email-masked]>
Date: Fri, 07 Oct 2022 10:03:50 +0300

> Cc: [email-masked]
> Date: Fri, 07 Oct 2022 08:46:15 +0800
> From:  Po Lu via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <[email-masked]>
> 
> Gerd Möllmann <[email-masked]> writes:
> 
> >     #0 0x1033f2ca8 in wrap_malloc+0x94 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x3eca8)
> >     #1 0x1005af4f4 in lmalloc alloc.c:1361
> >     #2 0x1005af40c in xmalloc alloc.c:751
> >     #3 0x1003f92b4 in make_realized_face xfaces.c:4471
> >     #4 0x1003f5c00 in realize_gui_face xfaces.c:6023
> >     #5 0x1003e4000 in realize_face xfaces.c:5954
> 
> [...]
> 
> >     #14 0x1005592d8 in Fvertical_motion indent.c:2241
> 
> I'm pretty sure the right fix is to block input around realize_face and
> Fvertical_motion, since that code is clearly not reentrant.

Why isn't Fvertical_motion reentrant?

Anyway, the problem is not that realize_face was interrupted, the
problem is that the face realized above was later freed as a side
effect of calling redisplay.  And the display code (which is invoked
by Fvertical_motion) almost everywhere assumes that FACE_FROM_ID will
never yield a freed face, it just returns

   FRAME_FACE_CACHE (f)->faces_by_id[id]

without checking whether ID is beyond the limit of the frame's current
face cache.  The assertion there is not compiled in a production
build.  (Gerd, was your build with --enable-checking?)

So if the frame's face cache can be freed like that as a side effect
of maybe_quit, we'll have to introduce cache checking into
FACE_FROM_ID, and if the ID is not in the cache do whatever it takes
to correct the situation.

IOW, I don't see how block_input anywhere can solve this particular
problem.