head -r320482 (e.g.): DNINIT_ALL and SOCK_MAXADDRLEN vs. SOCK_MAXADDRLEN+1 use in char buf[<?>] declarations: is the usage all correct?

[Mark Millard]

While trying to track down a repeatable crash
for powerpc production kernel's (but not
debug ones) I ran into the following. (I'm
not familiar with the material involved.)

NDINIT_ALL in /usr/src/sys/kern/vfs_lookup.c
records its char* namep argument in ndp->ni_dirp :

/usr/src/sys/kern/uipc_usrreq.c:
        char buf[SOCK_MAXADDRLEN];
. . .
        NDINIT_ATRIGHTS(&nd, LOOKUP, FOLLOW | LOCKSHARED | LOCKLEAF,
            UIO_SYSSPACE, buf, fd, cap_rights_init(&rights, CAP_CONNECTAT), td);
. . .
(elsewhere:)
void
NDINIT_ALL(struct nameidata *ndp, u_long op, u_long flags, enum uio_seg segflg,
    const char *namep, int dirfd, struct vnode *startdir, cap_rights_t *rightsp,
    struct thread *td)
{

        ndp->ni_cnd.cn_nameiop = op;
        ndp->ni_cnd.cn_flags = flags;
        ndp->ni_segflg = segflg;
        ndp->ni_dirp = namep;
        ndp->ni_dirfd = dirfd;
        ndp->ni_startdir = startdir;
        if (rightsp != NULL)
                ndp->ni_rightsneeded = *rightsp;
        else
                cap_rights_init(&ndp->ni_rightsneeded);
        filecaps_init(&ndp->ni_filecaps);
        ndp->ni_cnd.cn_thread = td;
}

But the size of the buffer supplied in other calls varies
from the size used for this call:

/usr/src/sys/fs/nfsclient/nfs_clvfsops.c:                       if (args.addrlen > SOCK_MAXADDRLEN) {
/usr/src/sys/kern/uipc_syscalls.c:      if (len > SOCK_MAXADDRLEN)
/usr/src/sys/kern/uipc_usrreq.c:        char buf[SOCK_MAXADDRLEN];
/usr/src/sys/netgraph/ng_ksocket.c:             if (pathlen > SOCK_MAXADDRLEN) {
/usr/src/sys/netgraph/ng_ksocket.c:             char pathbuf[SOCK_MAXADDRLEN + 1];
/usr/src/sys/sys/socket.h:#define       SOCK_MAXADDRLEN 255             /* longest possible addresses */

Some of the other usage of SOCK_MAXADDRLEN is:

                if (vfs_getopt(mp->mnt_optnew, "addr",
                    (void **)&args.addr, &args.addrlen) == 0) {
                        if (args.addrlen > SOCK_MAXADDRLEN) {
                                error = ENAMETOOLONG;
                                goto out;
                        }
                        nam = malloc(args.addrlen, M_SONAME, M_WAITOK);
                        bcopy(args.addr, nam, args.addrlen);
                        nam->sa_len = args.addrlen;
. . .
        if (len > SOCK_MAXADDRLEN)
                return (ENAMETOOLONG);
        if (len < offsetof(struct sockaddr, sa_data[0]))
                return (EINVAL);
        sa = malloc(len, M_SONAME, M_WAITOK);
. . .
                if ((path = ng_get_string_token(s, off, &toklen, NULL)) == NULL)
                        return (EINVAL);
                pathlen = strlen(path);
                if (pathlen > SOCK_MAXADDRLEN) {
                        free(path, M_NETGRAPH_KSOCKET);
                        return (E2BIG);
                }
                if (*buflen < pathoff + pathlen) {
                        free(path, M_NETGRAPH_KSOCKET);
                        return (ERANGE);
                }
                *off += toklen;
                bcopy(path, sun->sun_path, pathlen);

That last suggests that pathlen does not include a '\0'
termination character position but that path is supposed
to be '\0' terminated (in order to justify strlen(.) use)
yet the later bcopy avoids the '\0' position for
sun->sun_path.

This makes it unclear which of the following is
more appropriate vs. if each is correct for its
specific usage:

/usr/src/sys/kern/uipc_usrreq.c:
. . .
        char buf[SOCK_MAXADDRLEN];
. . .
        NDINIT_ATRIGHTS(&nd, LOOKUP, FOLLOW | LOCKSHARED | LOCKLEAF,
            UIO_SYSSPACE, buf, fd, cap_rights_init(&rights, CAP_CONNECTAT), td);

(The above buf has an odd number of bytes.)

(The above code is in the execution sequence that leads
to the powerpc fatal trap.)

/usr/src/sys/netgraph/ng_ksocket.c:
. . .
        case PF_LOCAL:
            {
                const int pathoff = OFFSETOF(struct sockaddr_un, sun_path);
                const struct sockaddr_un *sun = (const struct sockaddr_un *)sa;
                const int pathlen = sun->sun_len - pathoff;
                char pathbuf[SOCK_MAXADDRLEN + 1];
                char *pathtoken;

                bcopy(sun->sun_path, pathbuf, pathlen);
                if ((pathtoken = ng_encode_string(pathbuf, pathlen)) == NULL)
                        return (ENOMEM);
                slen += snprintf(cbuf, cbuflen, "local/%s", pathtoken);
                free(pathtoken, M_NETGRAPH_KSOCKET);
                if (slen >= cbuflen)
                        return (ERANGE);
                *off += sun->sun_len;
                return (0);
            }


===
Mark Millard
markmi at dsl-only.net