Download raw body.
vi(1), adding sizeof() to malloc when using CHAR_T
On Mon, Apr 20, 2026 at 12:17:16PM +0200, Walter Alejandro Iglesias wrote:
> Hunting the cause of this bug in our vi(1):
>
> https://marc.info/?l=openbsd-misc&m=177649923622612&w=2
>
> I compiled nvi2 from ports on linux to be able to use valgrind. The
> output at bottom shows a bunch "Invalid write" that happens after
> removing the '* sizeof(CHAR_T)' from MALLOC_RET (which is already added
> in this version) in ex_g_setup() (ex/ex_global.c:172). The diff below
> adds that same multiplication in other places where CHAR_T is used.
>
> I don't know if this affects OpenBSD's malloc in the same manner, but I
> guess it doesn't hurt to add it:
For OpenBSD CHAR_T is uchar_t, which is 1 by definition. So I'm not
sure this makes sense as long as we keep it that way.
-Otto
>
>
> Index: common/util.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/vi/common/util.c,v
> diff -u -p -u -p -r1.17 util.c
> --- common/util.c 11 Jul 2018 06:39:23 -0000 1.17
> +++ common/util.c 7 Apr 2026 16:27:43 -0000
> @@ -103,7 +103,7 @@ v_strdup(SCR *sp, const CHAR_T *str, siz
> {
> CHAR_T *copy;
>
> - MALLOC(sp, copy, len + 1);
> + MALLOC(sp, copy, (len + 1) * sizeof(CHAR_T));
> if (copy == NULL)
> return (NULL);
> memcpy(copy, str, len * sizeof(CHAR_T));
> Index: ex/ex_at.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/vi/ex/ex_at.c,v
> diff -u -p -u -p -r1.14 ex_at.c
> --- ex/ex_at.c 27 May 2016 09:18:12 -0000 1.14
> +++ ex/ex_at.c 7 Apr 2026 16:27:43 -0000
> @@ -105,7 +105,7 @@ ex_at(SCR *sp, EXCMD *cmdp)
> len += tp->len + 1;
> }
>
> - MALLOC_RET(sp, ecp->cp, len * 2);
> + MALLOC_RET(sp, ecp->cp, len * 2 * sizeof(CHAR_T));
> ecp->o_cp = ecp->cp;
> ecp->o_clen = len;
> ecp->cp[len] = '\0';
> Index: ex/ex_global.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/vi/ex/ex_global.c,v
> diff -u -p -u -p -r1.17 ex_global.c
> --- ex/ex_global.c 27 May 2016 09:18:12 -0000 1.17
> +++ ex/ex_global.c 7 Apr 2026 16:27:43 -0000
> @@ -170,7 +170,7 @@ usage: ex_emsg(sp, cmdp->cmd->usage, EX
> len = 1;
> }
>
> - MALLOC_RET(sp, ecp->cp, len * 2);
> + MALLOC_RET(sp, ecp->cp, len * 2 * sizeof(CHAR_T));
> ecp->o_cp = ecp->cp;
> ecp->o_clen = len;
> memcpy(ecp->cp + len, p, len);
>
>
>
> ==12304== Memcheck, a memory error detector
> ==12304== Copyright (C) 2002-2024, and GNU GPL'd, by Julian Seward et al.
> ==12304== Using Valgrind-3.24.0 and LibVEX; rerun with -h for copyright info
> ==12304== Command: nvi2/nvi -e foo
> ==12304== Parent PID: 8077
> ==12304==
> ==12304== Invalid write of size 2
> ==12304== at 0x48525C3: memmove (vg_replace_strmem.c:1414)
> ==12304== by 0x141D27: ex_g_setup (ex_global.c:175)
> ==12304== by 0x1417E4: ex_global (ex_global.c:40)
> ==12304== by 0x135B14: ex_cmd (ex.c:1386)
> ==12304== by 0x132655: ex (ex.c:127)
> ==12304== by 0x124993: editor (main.c:402)
> ==12304== by 0x115766: main (cl_main.c:115)
> ==12304== Address 0x4b3bf18 is 4 bytes after a block of size 4 alloc'd
> ==12304== at 0x4844818: malloc (vg_replace_malloc.c:446)
> ==12304== by 0x141C97: ex_g_setup (ex_global.c:172)
> ==12304== by 0x1417E4: ex_global (ex_global.c:40)
> ==12304== by 0x135B14: ex_cmd (ex.c:1386)
> ==12304== by 0x132655: ex (ex.c:127)
> ==12304== by 0x124993: editor (main.c:402)
> ==12304== by 0x115766: main (cl_main.c:115)
> ==12304==
> ==12304== Invalid read of size 8
> ==12304== at 0x48526E1: memmove (vg_replace_strmem.c:1414)
> ==12304== by 0x13772B: ex_load (ex.c:2160)
> ==12304== by 0x132C52: ex_cmd (ex.c:322)
> ==12304== by 0x132655: ex (ex.c:127)
> ==12304== by 0x124993: editor (main.c:402)
> ==12304== by 0x115766: main (cl_main.c:115)
> ==12304== Address 0x4b3bf18 is 4 bytes after a block of size 4 alloc'd
> ==12304== at 0x4844818: malloc (vg_replace_malloc.c:446)
> ==12304== by 0x141C97: ex_g_setup (ex_global.c:172)
> ==12304== by 0x1417E4: ex_global (ex_global.c:40)
> ==12304== by 0x135B14: ex_cmd (ex.c:1386)
> ==12304== by 0x132655: ex (ex.c:127)
> ==12304== by 0x124993: editor (main.c:402)
> ==12304== by 0x115766: main (cl_main.c:115)
> ==12304==
> ==12304== Invalid write of size 8
> ==12304== at 0x48526E4: memmove (vg_replace_strmem.c:1414)
> ==12304== by 0x13772B: ex_load (ex.c:2160)
> ==12304== by 0x132C52: ex_cmd (ex.c:322)
> ==12304== by 0x132655: ex (ex.c:127)
> ==12304== by 0x124993: editor (main.c:402)
> ==12304== by 0x115766: main (cl_main.c:115)
> ==12304== Address 0x4b3bf10 is 0 bytes inside a block of size 4 alloc'd
> ==12304== at 0x4844818: malloc (vg_replace_malloc.c:446)
> ==12304== by 0x141C97: ex_g_setup (ex_global.c:172)
> ==12304== by 0x1417E4: ex_global (ex_global.c:40)
> ==12304== by 0x135B14: ex_cmd (ex.c:1386)
> ==12304== by 0x132655: ex (ex.c:127)
> ==12304== by 0x124993: editor (main.c:402)
> ==12304== by 0x115766: main (cl_main.c:115)
> ==12304==
> ==12304== Invalid read of size 4
> ==12304== at 0x132E27: ex_cmd (ex.c:391)
> ==12304== by 0x132655: ex (ex.c:127)
> ==12304== by 0x124993: editor (main.c:402)
> ==12304== by 0x115766: main (cl_main.c:115)
> ==12304== Address 0x4b3bf14 is 0 bytes after a block of size 4 alloc'd
> ==12304== at 0x4844818: malloc (vg_replace_malloc.c:446)
> ==12304== by 0x141C97: ex_g_setup (ex_global.c:172)
> ==12304== by 0x1417E4: ex_global (ex_global.c:40)
> ==12304== by 0x135B14: ex_cmd (ex.c:1386)
> ==12304== by 0x132655: ex (ex.c:127)
> ==12304== by 0x124993: editor (main.c:402)
> ==12304== by 0x115766: main (cl_main.c:115)
> ==12304==
> ==12304== Conditional jump or move depends on uninitialised value(s)
> ==12304== at 0x4851B9E: bcmp (vg_replace_strmem.c:1233)
> ==12304== by 0x137C2F: ex_comm_search (ex.c:2283)
> ==12304== by 0x133072: ex_cmd (ex.c:454)
> ==12304== by 0x132655: ex (ex.c:127)
> ==12304== by 0x124993: editor (main.c:402)
> ==12304== by 0x115766: main (cl_main.c:115)
> ==12304==
> ==12304== Invalid read of size 1
> ==12304== at 0x4851B69: bcmp (vg_replace_strmem.c:1233)
> ==12304== by 0x137C2F: ex_comm_search (ex.c:2283)
> ==12304== by 0x133072: ex_cmd (ex.c:454)
> ==12304== by 0x132655: ex (ex.c:127)
> ==12304== by 0x124993: editor (main.c:402)
> ==12304== by 0x115766: main (cl_main.c:115)
> ==12304== Address 0x4b3bf14 is 0 bytes after a block of size 4 alloc'd
> ==12304== at 0x4844818: malloc (vg_replace_malloc.c:446)
> ==12304== by 0x141C97: ex_g_setup (ex_global.c:172)
> ==12304== by 0x1417E4: ex_global (ex_global.c:40)
> ==12304== by 0x135B14: ex_cmd (ex.c:1386)
> ==12304== by 0x132655: ex (ex.c:127)
> ==12304== by 0x124993: editor (main.c:402)
> ==12304== by 0x115766: main (cl_main.c:115)
> ==12304==
> ==12304== Invalid read of size 8
> ==12304== at 0x48526E1: memmove (vg_replace_strmem.c:1414)
> ==12304== by 0x13772B: ex_load (ex.c:2160)
> ==12304== by 0x132C52: ex_cmd (ex.c:322)
> ==12304== by 0x1571C4: v_ex (v_ex.c:401)
> ==12304== by 0x169E37: vi (vi.c:226)
> ==12304== by 0x154152: ex_visual (ex_visual.c:136)
> ==12304== by 0x135B14: ex_cmd (ex.c:1386)
> ==12304== by 0x132655: ex (ex.c:127)
> ==12304== by 0x124993: editor (main.c:402)
> ==12304== by 0x115766: main (cl_main.c:115)
> ==12304== Address 0x4b3bf18 is 4 bytes after a block of size 4 alloc'd
> ==12304== at 0x4844818: malloc (vg_replace_malloc.c:446)
> ==12304== by 0x141C97: ex_g_setup (ex_global.c:172)
> ==12304== by 0x1417E4: ex_global (ex_global.c:40)
> ==12304== by 0x135B14: ex_cmd (ex.c:1386)
> ==12304== by 0x132655: ex (ex.c:127)
> ==12304== by 0x124993: editor (main.c:402)
> ==12304== by 0x115766: main (cl_main.c:115)
> ==12304==
> ==12304== Invalid write of size 8
> ==12304== at 0x48526E4: memmove (vg_replace_strmem.c:1414)
> ==12304== by 0x13772B: ex_load (ex.c:2160)
> ==12304== by 0x132C52: ex_cmd (ex.c:322)
> ==12304== by 0x1571C4: v_ex (v_ex.c:401)
> ==12304== by 0x169E37: vi (vi.c:226)
> ==12304== by 0x154152: ex_visual (ex_visual.c:136)
> ==12304== by 0x135B14: ex_cmd (ex.c:1386)
> ==12304== by 0x132655: ex (ex.c:127)
> ==12304== by 0x124993: editor (main.c:402)
> ==12304== by 0x115766: main (cl_main.c:115)
> ==12304== Address 0x4b3bf10 is 0 bytes inside a block of size 4 alloc'd
> ==12304== at 0x4844818: malloc (vg_replace_malloc.c:446)
> ==12304== by 0x141C97: ex_g_setup (ex_global.c:172)
> ==12304== by 0x1417E4: ex_global (ex_global.c:40)
> ==12304== by 0x135B14: ex_cmd (ex.c:1386)
> ==12304== by 0x132655: ex (ex.c:127)
> ==12304== by 0x124993: editor (main.c:402)
> ==12304== by 0x115766: main (cl_main.c:115)
> ==12304==
> ==12304== Invalid read of size 4
> ==12304== at 0x132E27: ex_cmd (ex.c:391)
> ==12304== by 0x1571C4: v_ex (v_ex.c:401)
> ==12304== by 0x169E37: vi (vi.c:226)
> ==12304== by 0x154152: ex_visual (ex_visual.c:136)
> ==12304== by 0x135B14: ex_cmd (ex.c:1386)
> ==12304== by 0x132655: ex (ex.c:127)
> ==12304== by 0x124993: editor (main.c:402)
> ==12304== by 0x115766: main (cl_main.c:115)
> ==12304== Address 0x4b3bf14 is 0 bytes after a block of size 4 alloc'd
> ==12304== at 0x4844818: malloc (vg_replace_malloc.c:446)
> ==12304== by 0x141C97: ex_g_setup (ex_global.c:172)
> ==12304== by 0x1417E4: ex_global (ex_global.c:40)
> ==12304== by 0x135B14: ex_cmd (ex.c:1386)
> ==12304== by 0x132655: ex (ex.c:127)
> ==12304== by 0x124993: editor (main.c:402)
> ==12304== by 0x115766: main (cl_main.c:115)
> ==12304==
> ==12304== Conditional jump or move depends on uninitialised value(s)
> ==12304== at 0x4851B9E: bcmp (vg_replace_strmem.c:1233)
> ==12304== by 0x137C2F: ex_comm_search (ex.c:2283)
> ==12304== by 0x133072: ex_cmd (ex.c:454)
> ==12304== by 0x1571C4: v_ex (v_ex.c:401)
> ==12304== by 0x169E37: vi (vi.c:226)
> ==12304== by 0x154152: ex_visual (ex_visual.c:136)
> ==12304== by 0x135B14: ex_cmd (ex.c:1386)
> ==12304== by 0x132655: ex (ex.c:127)
> ==12304== by 0x124993: editor (main.c:402)
> ==12304== by 0x115766: main (cl_main.c:115)
> ==12304==
> ==12304== Invalid read of size 1
> ==12304== at 0x4851B69: bcmp (vg_replace_strmem.c:1233)
> ==12304== by 0x137C2F: ex_comm_search (ex.c:2283)
> ==12304== by 0x133072: ex_cmd (ex.c:454)
> ==12304== by 0x1571C4: v_ex (v_ex.c:401)
> ==12304== by 0x169E37: vi (vi.c:226)
> ==12304== by 0x154152: ex_visual (ex_visual.c:136)
> ==12304== by 0x135B14: ex_cmd (ex.c:1386)
> ==12304== by 0x132655: ex (ex.c:127)
> ==12304== by 0x124993: editor (main.c:402)
> ==12304== by 0x115766: main (cl_main.c:115)
> ==12304== Address 0x4b3bf14 is 0 bytes after a block of size 4 alloc'd
> ==12304== at 0x4844818: malloc (vg_replace_malloc.c:446)
> ==12304== by 0x141C97: ex_g_setup (ex_global.c:172)
> ==12304== by 0x1417E4: ex_global (ex_global.c:40)
> ==12304== by 0x135B14: ex_cmd (ex.c:1386)
> ==12304== by 0x132655: ex (ex.c:127)
> ==12304== by 0x124993: editor (main.c:402)
> ==12304== by 0x115766: main (cl_main.c:115)
>
>
>
> --
> Walter
>
vi(1), adding sizeof() to malloc when using CHAR_T