On Thu, Feb 28, 2019 at 04:56:01AM +0300, magras wrote: > Current font caching algorithm contains a use after free error. A font > removed from `frc` might be still listed in `wx.specbuf`. It will lead > to a crash inside `XftDrawGlyphFontSpec()`. > > Steps to reproduce: > $ st -f 'Misc Tamsyn:scalable=false' > $ curl https://www.cl.cam.ac.uk/~mgk25/ucs/examples/UTF-8-demo.txt > > Of course, result depends on fonts installed on a system and fontconfig. > In my case, I'm getting consistent segfaults with different fonts. > > I replaced a fixed array with a simple unbounded buffer with a constant > growth rate. Cache starts with a capacity of 0, gets increments by 16, > and never shrinks. On my machine after `cat UTF-8-demo.txt` buffer > reaches a capacity of 192. During casual use capacity stays at 0. > --- > x.c | 15 +++++++++------ > 1 file changed, 9 insertions(+), 6 deletions(-) > > diff --git a/x.c b/x.c > index 865dacc..2cd76d0 100644 > --- a/x.c > +++ b/x.c > @@ -226,8 +226,9 @@ typedef struct { > } Fontcache; > > /* Fontcache is an array now. A new font will be appended to the array. */ > -static Fontcache frc[16]; > +static Fontcache *frc = NULL; > static int frclen = 0; > +static int frccap = 0; > static char *usedfont = NULL; > static double usedfontsize = 0; > static double defaultfontsize = 0; > @@ -1244,12 +1245,14 @@ xmakeglyphfontspecs(XftGlyphFontSpec *specs, const > Glyph *glyphs, int len, int x > fcpattern, &fcres); > > /* > - * Overwrite or create the new cache entry. > + * Allocate memory for the new cache entry. > */ > - if (frclen >= LEN(frc)) { > - frclen = LEN(frc) - 1; > - XftFontClose(xw.dpy, frc[frclen].font); > - frc[frclen].unicodep = 0; > + if (frclen >= frccap) { > + frccap += 16; > + if (!frc) > + frc = xmalloc(frccap * > sizeof(Fontcache)); > + else > + frc = xrealloc(frc, frccap * > sizeof(Fontcache)); > } > > frc[frclen].font = XftFontOpenPattern(xw.dpy, > -- > 2.21.0 > >
Pushed thanks. I have simplified the allocating in a subsequent commit. POSIX says: "If ptr is a null pointer, realloc() shall be equivalent to malloc() for the specified size." -- Kind regards, Hiltjo
