From owner-svn-src-projects@freebsd.org Sun Mar 17 18:35:36 2019 Return-Path: Delivered-To: svn-src-projects@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 906DE154689D for ; Sun, 17 Mar 2019 18:35:36 +0000 (UTC) (envelope-from jilles@stack.nl) Received: from ecc03.stack.nl (ecc03.stack.nl [IPv6:2001:610:1108:5010::210]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "*.stack.nl", Issuer "COMODO RSA Domain Validation Secure Server CA" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id C458C8B89E; Sun, 17 Mar 2019 18:35:35 +0000 (UTC) (envelope-from jilles@stack.nl) Received: from mail02.stack.nl (blade.stack.nl [51.15.111.152]) by ecc03.stack.nl (Postfix) with ESMTPS id AD411202BC; Sun, 17 Mar 2019 18:35:33 +0000 (UTC) Received: from localhost (localhost.localdomain [127.0.0.1]) by mail02.stack.nl (Postfix) with ESMTP id 7E168241798; Sun, 17 Mar 2019 18:35:33 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at mail02 Received: from mail02.stack.nl ([127.0.0.1]) by localhost (mail02.stack.nl [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id rHWeLPAvwORb; Sun, 17 Mar 2019 18:35:30 +0000 (UTC) Received: from blade.stack.nl (blade.stack.nl [192.168.122.130]) by mail02.stack.nl (Postfix) with ESMTP id 5191F241792; Sun, 17 Mar 2019 18:35:30 +0000 (UTC) Received: by blade.stack.nl (Postfix, from userid 1677) id 336C92057D; Sun, 17 Mar 2019 19:35:30 +0100 (CET) Date: Sun, 17 Mar 2019 19:35:30 +0100 From: Jilles Tjoelker To: Ian Lepore Cc: Enji Cooper , src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: Re: svn commit: r345215 - projects/capsicum-test/contrib/capsicum-test Message-ID: <20190317183530.GA10290@stack.nl> References: <201903160316.x2G3Gi2r072317@repo.freebsd.org> <20190317155340.GA3659@stack.nl> <563764a4fd41692467166a863e52e88bab030034.camel@freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <563764a4fd41692467166a863e52e88bab030034.camel@freebsd.org> User-Agent: Mutt/1.9.4 (2018-02-28) X-BeenThere: svn-src-projects@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the src " projects" tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 17 Mar 2019 18:35:36 -0000 On Sun, Mar 17, 2019 at 10:44:47AM -0600, Ian Lepore wrote: > On Sun, 2019-03-17 at 16:53 +0100, Jilles Tjoelker wrote: > > On Sat, Mar 16, 2019 at 03:16:44AM +0000, Enji Cooper wrote: > > > Author: ngie > > > Date: Sat Mar 16 03:16:44 2019 > > > New Revision: 345215 > > > URL: https://svnweb.freebsd.org/changeset/base/345215 > > > Log: > > > Revert r345214 > > > It's best to not repeat the mistake I made in r300167; I should use > > > `NO_WCAST_ALIGN` instead. > > > Modified: > > > projects/capsicum-test/contrib/capsicum-test/capability-fd.cc > > > Modified: projects/capsicum-test/contrib/capsicum-test/capability-fd.cc > > > ============================================================================== > > > --- projects/capsicum-test/contrib/capsicum-test/capability-fd.cc Sat Mar 16 03:03:25 2019 (r345214) > > > +++ projects/capsicum-test/contrib/capsicum-test/capability-fd.cc Sat Mar 16 03:16:44 2019 (r345215) > > > @@ -982,14 +982,12 @@ FORK_TEST_ON(Capability, SocketTransfer, TmpFile("cap_ > > > // Child: enter cap mode > > > EXPECT_OK(cap_enter()); > > > > > > - int cap_fd; > > > - > > > // Child: wait to receive FD over socket > > > int rc = recvmsg(sock_fds[0], &mh, 0); > > > EXPECT_OK(rc); > > > EXPECT_LE(CMSG_LEN(sizeof(int)), mh.msg_controllen); > > > cmptr = CMSG_FIRSTHDR(&mh); > > > - memcpy(&cap_fd, CMSG_DATA(cmptr), sizeof(int)); > > > + int cap_fd = *(int*)CMSG_DATA(cmptr); > > > EXPECT_EQ(CMSG_LEN(sizeof(int)), cmptr->cmsg_len); > > > cmptr = CMSG_NXTHDR(&mh, cmptr); > > > EXPECT_TRUE(cmptr == NULL); > > > @@ -1024,7 +1022,7 @@ FORK_TEST_ON(Capability, SocketTransfer, TmpFile("cap_ > > > cmptr->cmsg_level = SOL_SOCKET; > > > cmptr->cmsg_type = SCM_RIGHTS; > > > cmptr->cmsg_len = CMSG_LEN(sizeof(int)); > > > - memcpy(CMSG_DATA(cmptr), &cap_fd, sizeof(int)); > > > + *(int *)CMSG_DATA(cmptr) = cap_fd; > > > buffer1[0] = 0; > > > iov[0].iov_len = 1; > > > sleep(3); > > I think it is better to suppress the alignment warning via an > > intermediate (void *) cast than to suppress it more globally. > > As for other considerations between dereferencing cast pointers and > > memcpy(), the use of memcpy() also allows reading and writing regardless > > of the effective type of the object (strict aliasing). However, that is > > of limited use in this particular case since the CMSG_NXTHDR macro > > already dereferences a cast pointer -- the buffer pointed to by > > msg_control must have a suitable effective type. > > In this case, the buffer pointed to by msg_control is a char array, > > which may only be accessed using lvalues of type char, signed char, > > unsigned char or qualified versions of those. Additionally, it is not, > > in general, guaranteed to be suitably aligned (the amd64 ABI guarantees > > it, though). > I believe it is suitably aligned. When these values are constructed in > the kernel they use the _ALIGN() macro which is defined in > machine/_align.h and is supposed to give the appropriate alignment for > the platform for any type that can appear in the control-message area. The kernel ensures alignment of the different cmsghdr structs and their data fields (by adding padding bytes), under the assumption that the buffer pointed to by msg_control is suitably aligned. Also see the definition of CMSG_FIRSTHDR in : msg_control is cast to struct cmsghdr * without any alignment fixup. Therefore, the application must ensure that the msg_control buffer be properly aligned. If an application passes an unaligned buffer, the kernel's copyin and copyout will not care, but subsequent access to the control messages using the CMSG_ macros will be undefined behaviour (this might cause faults even on amd64 if the compiler vectorizes the code using SSE2). -- Jilles Tjoelker