From: Walter Alejandro Iglesias <wai@roquesor.com>
Subject: Re: vi(1), adding sizeof() to malloc when using CHAR_T
To: Theo Buehler <tb@theobuehler.org>
Cc: Otto Moerbeek <otto@drijf.net>, tech@openbsd.org
Date: Mon, 20 Apr 2026 13:20:30 +0200

On Mon, Apr 20, 2026 at 01:04:19PM +0200, Theo Buehler wrote:
> On Mon, Apr 20, 2026 at 12:53:00PM +0200, Otto Moerbeek wrote:
> > 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.
> 
> Well, as the diff shows, the code is rather confused about this.
> 
> 	MALLOC(sp, copy, len + 1);
> 	if (copy == NULL)
> 		return (NULL);
> 	memcpy(copy, str, len * sizeof(CHAR_T));
> 	copy[len] = '\0';
> 
> So just modifying the MALLOC lines doesn't really improve things anyway.
> Arguably, it would be beneficial to fix this one way or the other.

nvi2 from ports and debian 1.81.6, since they have added UTF-8 support,
have two functions, one for char and one for wide char:

char *
v_strdup(SCR *sp, const char *str, size_t len)
{
        char *copy;

        MALLOC(sp, copy, len + 1);
        if (copy == NULL)
                return (NULL);
        memcpy(copy, str, len);
        copy[len] = '\0';
        return (copy);
}

/*
 * v_wstrdup --
 *      Strdup for wide character strings with an associated length.
 *
 * PUBLIC: CHAR_T *v_wstrdup(SCR *, const CHAR_T *, size_t);
 */
CHAR_T *
v_wstrdup(SCR *sp, const CHAR_T *str, size_t len)
{
        CHAR_T *copy;

        MALLOC(sp, copy, (len + 1) * sizeof(CHAR_T));
        if (copy == NULL)
                return (NULL);
        MEMCPY(copy, str, len);
        copy[len] = '\0';
        return (copy);
}


If uchar_t is one bite then this should be fine for us?:


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	20 Apr 2026 11:14:25 -0000
@@ -106,7 +106,7 @@ v_strdup(SCR *sp, const CHAR_T *str, siz
 	MALLOC(sp, copy, len + 1);
 	if (copy == NULL)
 		return (NULL);
-	memcpy(copy, str, len * sizeof(CHAR_T));
+	memcpy(copy, str, len);
 	copy[len] = '\0';
 	return (copy);
 }


-- 
Walter