Skip site navigation (1)Skip section navigation (2) 
Date:      Tue, 26 Apr 2016 19:05:44 +0000
From:      bugzilla-noreply@freebsd.org
To:        freebsd-bugs@FreeBSD.org
Subject:   [Bug 209078] Minor bugs in vidcontrol
Message-ID:  <bug-209078-8@https.bugs.freebsd.org/bugzilla/>
next in thread | raw e-mail | index | archive | help 

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=209078

            Bug ID: 209078
           Summary: Minor bugs in vidcontrol
           Product: Base System
           Version: 11.0-CURRENT
          Hardware: Any
                OS: Any
            Status: New
          Severity: Affects Only Me
          Priority: ---
         Component: misc
          Assignee: freebsd-bugs@FreeBSD.org
          Reporter: cturt@hardenedbsd.org

There is a memory leak in the `vidcontrol` utility in the `load_vt4font`:

usr.sbin/vidcontrol/vidcontrol.c:

static int
load_vt4font(FILE *f)
{
        struct vt4font_header fh;
        static vfnt_t vfnt;
        size_t glyphsize;
        unsigned int i;

        if (fread(&fh, sizeof fh, 1, f) != 1) {
                perror("file_header");
                return (1);
        }

        if (memcmp(fh.magic, "VFNT0002", 8) != 0) {
                fprintf(stderr, "Bad magic\n");
                return (1);
        }

        for (i = 0; i < VFNT_MAPS; i++)
                vfnt.map_count[i] = be32toh(fh.map_count[i]);
        vfnt.glyph_count = be32toh(fh.glyph_count);
        vfnt.width = fh.width;
        vfnt.height = fh.height;

        glyphsize = howmany(vfnt.width, 8) * vfnt.height * vfnt.glyph_count;
        vfnt.glyphs = malloc(glyphsize);

        if (fread(vfnt.glyphs, glyphsize, 1, f) != 1) {
                perror("glyphs");
                return (1);
        }

        for (i = 0; i < VFNT_MAPS; i++)
                vfnt.map[i] = load_vt4mappingtable(vfnt.map_count[i], f);

        if (ioctl(STDIN_FILENO, PIO_VFONT, &vfnt) == -1) {
                perror("PIO_VFONT");
                return (1);
        }
        return (0);
}

After the `vfnt.glyphs` buffer has been allocated with `malloc`, the function
can return without freeing the buffer if `fread` or `ioctl` fail.

This is only a minor bug, since the process exits almost immediately after
calling this function anyway, but I would like to `free` the buffer as a matter
of code correctness.

This function also doesn't check the return result of `malloc`, which could
lead to writing to `NULL` if the allocation fails.

My proposal is to add the following lines to this function:

        vfnt.glyphs = malloc(glyphsize);
+       if (vfnt.glyphs == NULL) {
+               perror("malloc");
+               return (1);
+       }

        if (fread(vfnt.glyphs, glyphsize, 1, f) != 1) {
                perror("glyphs");
+               free(vfnt.glyphs);
                return (1);
        }

        for (i = 0; i < VFNT_MAPS; i++)
                vfnt.map[i] = load_vt4mappingtable(vfnt.map_count[i], f);

        if (ioctl(STDIN_FILENO, PIO_VFONT, &vfnt) == -1) {
                perror("PIO_VFONT");
+               free(vfnt.glyphs);
                return (1);
        }

-- 
You are receiving this mail because:
You are the assignee for the bug.

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?bug-209078-8>