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
> _AT_@ -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;
> _AT_@ -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
Received on Sun Mar 03 2019 - 11:37:34 CET