Date: Wed, 4 May 2016 07:37:02 +0000 (UTC) From: Garrett Cooper <ngie@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org Subject: svn commit: r299058 - stable/10/tests/sys/posixshm Message-ID: <201605040737.u447b2r8080678@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: ngie Date: Wed May 4 07:37:02 2016 New Revision: 299058 URL: https://svnweb.freebsd.org/changeset/base/299058 Log: MFC r298304: Fix issues identified by Coverity - Always munmap memory regions after mmap'ing them. - Make sure getpagesize() returns a value greater than 0 and use a cached value instead of always calling getpagesize(3). - Remove intermediate variable for assigning from $TMPDIR if set in the environment to eliminate warnings about pointer conversions with "/tmp", and to mute an invalid buffer overflow concern from Coverity (snprintf and tacking on a NUL terminator was alleviating that concern before). - Remove useless self-test of psize before it's initialized. - Check the return values of getrlimit/setrlimit. Cosmetic changes: - Replace a `(void*)0` with NULL. - Do some minor whitespace clean up. - Remove an unnecessary cast to mmap. - Make all munmap calls use ATF_REQUIRE_MSG instead of using the: > if (munmap(..) == -1) > atf_tc_fail(..) idiom. Employ the new idiom consistently when calling munmap. CID: 1331351, 1331382-1331386, 1331513, 1331514, 1331565, 1331583, 1331694 Modified: stable/10/tests/sys/posixshm/posixshm_test.c Directory Properties: stable/10/ (props changed) Modified: stable/10/tests/sys/posixshm/posixshm_test.c ============================================================================== --- stable/10/tests/sys/posixshm/posixshm_test.c Wed May 4 07:35:43 2016 (r299057) +++ stable/10/tests/sys/posixshm/posixshm_test.c Wed May 4 07:37:02 2016 (r299058) @@ -50,12 +50,9 @@ static char test_path[TEST_PATH_LEN]; static void gen_test_path(void) { - char *tmpdir = getenv("TMPDIR"); - if (tmpdir == NULL) - tmpdir = "/tmp"; - - snprintf(test_path, sizeof(test_path), "%s/tmp.XXXXXX", tmpdir); + snprintf(test_path, sizeof(test_path), "%s/tmp.XXXXXX", + getenv("TMPDIR") == NULL ? "/tmp" : getenv("TMPDIR")); test_path[sizeof(test_path) - 1] = '\0'; ATF_REQUIRE_MSG(mkstemp(test_path) != -1, "mkstemp failed; errno=%d", errno); @@ -99,10 +96,12 @@ static int scribble_object(void) { char *page; - int fd; + int fd, pagesize; gen_test_path(); + ATF_REQUIRE(0 < (pagesize = getpagesize())); + fd = shm_open(test_path, O_CREAT|O_EXCL|O_RDWR, 0777); if (fd < 0 && errno == EEXIST) { if (shm_unlink(test_path) < 0) @@ -111,17 +110,16 @@ scribble_object(void) } if (fd < 0) atf_tc_fail("shm_open failed; errno=%d", errno); - if (ftruncate(fd, getpagesize()) < 0) + if (ftruncate(fd, pagesize) < 0) atf_tc_fail("ftruncate failed; errno=%d", errno); - page = mmap(0, getpagesize(), PROT_READ | PROT_WRITE, MAP_SHARED, fd, - 0); + page = mmap(0, pagesize, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); if (page == MAP_FAILED) atf_tc_fail("mmap failed; errno=%d", errno); page[0] = '1'; - if (munmap(page, getpagesize()) < 0) - atf_tc_fail("munmap failed; errno=%d", errno); + ATF_REQUIRE_MSG(munmap(page, pagesize) == 0, "munmap failed; errno=%d", + errno); return (fd); } @@ -130,12 +128,13 @@ ATF_TC_WITHOUT_HEAD(remap_object); ATF_TC_BODY(remap_object, tc) { char *page; - int fd; + int fd, pagesize; + + ATF_REQUIRE(0 < (pagesize = getpagesize())); fd = scribble_object(); - page = mmap(0, getpagesize(), PROT_READ | PROT_WRITE, MAP_SHARED, fd, - 0); + page = mmap(0, pagesize, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); if (page == MAP_FAILED) atf_tc_fail("mmap(2) failed; errno=%d", errno); @@ -143,8 +142,8 @@ ATF_TC_BODY(remap_object, tc) atf_tc_fail("missing data ('%c' != '1')", page[0]); close(fd); - if (munmap(page, getpagesize()) < 0) - atf_tc_fail("munmap failed; errno=%d", errno); + ATF_REQUIRE_MSG(munmap(page, pagesize) == 0, "munmap failed; errno=%d", + errno); ATF_REQUIRE_MSG(shm_unlink(test_path) != -1, "shm_unlink failed; errno=%d", errno); @@ -154,7 +153,9 @@ ATF_TC_WITHOUT_HEAD(reopen_object); ATF_TC_BODY(reopen_object, tc) { char *page; - int fd; + int fd, pagesize; + + ATF_REQUIRE(0 < (pagesize = getpagesize())); fd = scribble_object(); close(fd); @@ -163,14 +164,15 @@ ATF_TC_BODY(reopen_object, tc) if (fd < 0) atf_tc_fail("shm_open(2) failed; errno=%d", errno); - page = mmap(0, getpagesize(), PROT_READ, MAP_SHARED, fd, 0); + page = mmap(0, pagesize, PROT_READ, MAP_SHARED, fd, 0); if (page == MAP_FAILED) atf_tc_fail("mmap(2) failed; errno=%d", errno); if (page[0] != '1') atf_tc_fail("missing data ('%c' != '1')", page[0]); - munmap(page, getpagesize()); + ATF_REQUIRE_MSG(munmap(page, pagesize) == 0, "munmap failed; errno=%d", + errno); close(fd); ATF_REQUIRE_MSG(shm_unlink(test_path) != -1, "shm_unlink failed; errno=%d", errno); @@ -180,7 +182,9 @@ ATF_TC_WITHOUT_HEAD(readonly_mmap_write) ATF_TC_BODY(readonly_mmap_write, tc) { char *page; - int fd; + int fd, pagesize; + + ATF_REQUIRE(0 < (pagesize = getpagesize())); gen_test_path(); @@ -188,8 +192,7 @@ ATF_TC_BODY(readonly_mmap_write, tc) ATF_REQUIRE_MSG(fd >= 0, "shm_open failed; errno=%d", errno); /* PROT_WRITE should fail with EACCES. */ - page = mmap(0, getpagesize(), PROT_READ | PROT_WRITE, MAP_SHARED, fd, - 0); + page = mmap(0, pagesize, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); if (page != MAP_FAILED) atf_tc_fail("mmap(PROT_WRITE) succeeded unexpectedly"); @@ -359,49 +362,49 @@ ATF_TC_BODY(object_resize, tc) { pid_t pid; struct stat sb; - char err_buf[1024], *page; - int fd, status; + char *page; + int fd, pagesize, status; + + ATF_REQUIRE(0 < (pagesize = getpagesize())); /* Start off with a size of a single page. */ fd = shm_open(SHM_ANON, O_CREAT|O_RDWR, 0777); if (fd < 0) atf_tc_fail("shm_open failed; errno=%d", errno); - if (ftruncate(fd, getpagesize()) < 0) + if (ftruncate(fd, pagesize) < 0) atf_tc_fail("ftruncate(1) failed; errno=%d", errno); if (fstat(fd, &sb) < 0) atf_tc_fail("fstat(1) failed; errno=%d", errno); - if (sb.st_size != getpagesize()) + if (sb.st_size != pagesize) atf_tc_fail("first resize failed (%d != %d)", - (int)sb.st_size, getpagesize()); + (int)sb.st_size, pagesize); /* Write a '1' to the first byte. */ - page = mmap(0, getpagesize(), PROT_READ|PROT_WRITE, MAP_SHARED, fd, - 0); + page = mmap(0, pagesize, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0); if (page == MAP_FAILED) atf_tc_fail("mmap(1)"); page[0] = '1'; - if (munmap(page, getpagesize()) < 0) - atf_tc_fail("munmap(1) failed; errno=%d", errno); + ATF_REQUIRE_MSG(munmap(page, pagesize) == 0, "munmap failed; errno=%d", + errno); /* Grow the object to 2 pages. */ - if (ftruncate(fd, getpagesize() * 2) < 0) + if (ftruncate(fd, pagesize * 2) < 0) atf_tc_fail("ftruncate(2) failed; errno=%d", errno); if (fstat(fd, &sb) < 0) atf_tc_fail("fstat(2) failed; errno=%d", errno); - if (sb.st_size != getpagesize() * 2) + if (sb.st_size != pagesize * 2) atf_tc_fail("second resize failed (%d != %d)", - (int)sb.st_size, getpagesize() * 2); + (int)sb.st_size, pagesize * 2); /* Check for '1' at the first byte. */ - page = mmap(0, getpagesize() * 2, PROT_READ|PROT_WRITE, MAP_SHARED, - fd, 0); + page = mmap(0, pagesize * 2, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0); if (page == MAP_FAILED) atf_tc_fail("mmap(2) failed; errno=%d", errno); @@ -409,18 +412,18 @@ ATF_TC_BODY(object_resize, tc) atf_tc_fail("'%c' != '1'", page[0]); /* Write a '2' at the start of the second page. */ - page[getpagesize()] = '2'; + page[pagesize] = '2'; /* Shrink the object back to 1 page. */ - if (ftruncate(fd, getpagesize()) < 0) + if (ftruncate(fd, pagesize) < 0) atf_tc_fail("ftruncate(3) failed; errno=%d", errno); if (fstat(fd, &sb) < 0) atf_tc_fail("fstat(3) failed; errno=%d", errno); - if (sb.st_size != getpagesize()) + if (sb.st_size != pagesize) atf_tc_fail("third resize failed (%d != %d)", - (int)sb.st_size, getpagesize()); + (int)sb.st_size, pagesize); /* * Fork a child process to make sure the second page is no @@ -435,16 +438,16 @@ ATF_TC_BODY(object_resize, tc) char c; /* Don't generate a core dump. */ - getrlimit(RLIMIT_CORE, &lim); + ATF_REQUIRE(getrlimit(RLIMIT_CORE, &lim) == 0); lim.rlim_cur = 0; - setrlimit(RLIMIT_CORE, &lim); + ATF_REQUIRE(setrlimit(RLIMIT_CORE, &lim) == 0); /* * The previous ftruncate(2) shrunk the backing object * so that this address is no longer valid, so reading * from it should trigger a SIGSEGV. */ - c = page[getpagesize()]; + c = page[pagesize]; fprintf(stderr, "child: page 1: '%c'\n", c); exit(0); } @@ -456,15 +459,15 @@ ATF_TC_BODY(object_resize, tc) atf_tc_fail("child terminated with status %x", status); /* Grow the object back to 2 pages. */ - if (ftruncate(fd, getpagesize() * 2) < 0) + if (ftruncate(fd, pagesize * 2) < 0) atf_tc_fail("ftruncate(2) failed; errno=%d", errno); if (fstat(fd, &sb) < 0) atf_tc_fail("fstat(2) failed; errno=%d", errno); - if (sb.st_size != getpagesize() * 2) + if (sb.st_size != pagesize * 2) atf_tc_fail("fourth resize failed (%d != %d)", - (int)sb.st_size, getpagesize()); + (int)sb.st_size, pagesize); /* * Note that the mapping at 'page' for the second page is @@ -475,9 +478,9 @@ ATF_TC_BODY(object_resize, tc) * object was shrunk and the new pages when an object are * grown are zero-filled. */ - if (page[getpagesize()] != 0) + if (page[pagesize] != 0) atf_tc_fail("invalid data at %d: %x != 0", - getpagesize(), (int)page[getpagesize()]); + pagesize, (int)page[pagesize]); close(fd); } @@ -524,7 +527,7 @@ ATF_TC_BODY(shm_functionality_across_for scval = sysconf(_SC_PAGESIZE); if (scval == -1 && errno != 0) { atf_tc_fail("sysconf(_SC_PAGESIZE) failed; errno=%d", errno); - } else if (scval <= 0 || (size_t)psize != psize) { + } else if (scval <= 0) { fprintf(stderr, "bogus return from sysconf(_SC_PAGESIZE): %ld", scval); psize = 4096; @@ -542,8 +545,7 @@ ATF_TC_BODY(shm_functionality_across_for ATF_REQUIRE_MSG(ftruncate(desc, (off_t)psize) != -1, "ftruncate failed; errno=%d", errno); - region = mmap((void *)0, psize, PROT_READ | PROT_WRITE, MAP_SHARED, - desc, (off_t)0); + region = mmap(NULL, psize, PROT_READ | PROT_WRITE, MAP_SHARED, desc, 0); ATF_REQUIRE_MSG(region != MAP_FAILED, "mmap failed; errno=%d", errno); memset(region, '\377', psize); @@ -601,6 +603,10 @@ ATF_TC_BODY(shm_functionality_across_for strsignal(WTERMSIG(status))); } } + + ATF_REQUIRE_MSG(munmap(region, psize) == 0, "munmap failed; errno=%d", + errno); + shm_unlink(test_path); } ATF_TP_ADD_TCS(tp)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201605040737.u447b2r8080678>