From owner-freebsd-hackers@freebsd.org Sat Jul 1 00:06:05 2017 Return-Path: Delivered-To: freebsd-hackers@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id EF1BAD9E2C8 for ; Sat, 1 Jul 2017 00:06:05 +0000 (UTC) (envelope-from markmi@dsl-only.net) Received: from asp.reflexion.net (outbound-mail-210-15.reflexion.net [208.70.210.15]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id A8E1A837C2 for ; Sat, 1 Jul 2017 00:06:04 +0000 (UTC) (envelope-from markmi@dsl-only.net) Received: (qmail 14351 invoked from network); 1 Jul 2017 00:05:58 -0000 Received: from unknown (HELO mail-cs-01.app.dca.reflexion.local) (10.81.19.1) by 0 (rfx-qmail) with SMTP; 1 Jul 2017 00:05:58 -0000 Received: by mail-cs-01.app.dca.reflexion.local (Reflexion email security v8.40.1) with SMTP; Fri, 30 Jun 2017 20:05:58 -0400 (EDT) Received: (qmail 23420 invoked from network); 1 Jul 2017 00:05:58 -0000 Received: from unknown (HELO iron2.pdx.net) (69.64.224.71) by 0 (rfx-qmail) with (AES256-SHA encrypted) SMTP; 1 Jul 2017 00:05:58 -0000 Received: from [192.168.1.114] (c-76-115-7-162.hsd1.or.comcast.net [76.115.7.162]) by iron2.pdx.net (Postfix) with ESMTPSA id 5280AEC893D; Fri, 30 Jun 2017 17:05:57 -0700 (PDT) From: Mark Millard Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: quoted-printable Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\)) Subject: head -r320482 (e.g.): DNINIT_ALL and SOCK_MAXADDRLEN vs. SOCK_MAXADDRLEN+1 use in char buf[] declarations: is the usage all correct? Message-Id: <3390B896-622E-4548-B483-5772AB3ABD0C@dsl-only.net> Date: Fri, 30 Jun 2017 17:05:56 -0700 To: freebsd-hackers@freebsd.org, FreeBSD PowerPC ML X-Mailer: Apple Mail (2.3273) X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 01 Jul 2017 00:06:06 -0000 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 =3D op; ndp->ni_cnd.cn_flags =3D flags; ndp->ni_segflg =3D segflg; ndp->ni_dirp =3D namep; ndp->ni_dirfd =3D dirfd; ndp->ni_startdir =3D startdir; if (rightsp !=3D NULL) ndp->ni_rightsneeded =3D *rightsp; else cap_rights_init(&ndp->ni_rightsneeded); filecaps_init(&ndp->ni_filecaps); ndp->ni_cnd.cn_thread =3D 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) =3D=3D 0) { if (args.addrlen > SOCK_MAXADDRLEN) { error =3D ENAMETOOLONG; goto out; } nam =3D malloc(args.addrlen, M_SONAME, = M_WAITOK); bcopy(args.addr, nam, args.addrlen); nam->sa_len =3D args.addrlen; . . . if (len > SOCK_MAXADDRLEN) return (ENAMETOOLONG); if (len < offsetof(struct sockaddr, sa_data[0])) return (EINVAL); sa =3D malloc(len, M_SONAME, M_WAITOK); . . . if ((path =3D ng_get_string_token(s, off, &toklen, = NULL)) =3D=3D NULL) return (EINVAL); pathlen =3D 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 +=3D 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 =3D OFFSETOF(struct sockaddr_un, = sun_path); const struct sockaddr_un *sun =3D (const struct = sockaddr_un *)sa; const int pathlen =3D sun->sun_len - pathoff; char pathbuf[SOCK_MAXADDRLEN + 1]; char *pathtoken; bcopy(sun->sun_path, pathbuf, pathlen); if ((pathtoken =3D ng_encode_string(pathbuf, pathlen)) = =3D=3D NULL) return (ENOMEM); slen +=3D snprintf(cbuf, cbuflen, "local/%s", = pathtoken); free(pathtoken, M_NETGRAPH_KSOCKET); if (slen >=3D cbuflen) return (ERANGE); *off +=3D sun->sun_len; return (0); } =3D=3D=3D Mark Millard markmi at dsl-only.net