From mboxrd@z Thu Jan 1 00:00:00 1970 From: Cyril Hrubis Date: Wed, 26 Apr 2017 20:55:49 +0200 Subject: [LTP] [PATCH] mmapstress04: rewrite to fix heap overwrite In-Reply-To: References: Message-ID: <20170426185549.GC6165@rei.suse.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: ltp@lists.linux.it Hi! > +/* > + * Copyright (c) 2017 Red Hat, Inc. > + * 01/02/2003 Port to LTP avenkat@us.ibm.com > + * 06/30/2001 Port to Linux nsharoff@us.ibm.com > + * > + * This program is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation, either version 3 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see . > + */ > +/* > + * Mmap parts of a file, and then write to the file causing single > + * write requests to jump back and forth between mmaped io and regular io. > + */ > + > +#define _GNU_SOURCE > +#include > +#include "tst_test.h" > +#include "tst_safe_macros.h" > + > +#define NUM_PAGES (64*8) > +#define TEST_FILE "mmapstress04-testfile" > + > +static int page_size; > +static unsigned char *mmap_area; > + > +static void setup(void) > +{ > + page_size = getpagesize(); > + > + /* > + * Pick large enough area, PROT_NONE doesn't matter, > + * because we remap it later. > + */ > + mmap_area = SAFE_MMAP(NULL, page_size * NUM_PAGES, PROT_NONE, > + MAP_PRIVATE|MAP_ANONYMOUS, -1, 0); > +} > + > +static void write_fully(int fd, void *buf, int len) > +{ > + do { > + len -= SAFE_WRITE(0, fd, buf, len); Not that this matters here, since the buf is filled with 'a' bytes anyway, but we should probably add a offset to the buffer on subsequent writes or at least add a comment that we do not care. As it is the code looks buggy. > + } while (len > 0); > +} > + > +static void mmapstress04(void) > +{ > + int i, j, rofd, rwfd, ret = 0; > + char *buf; > + > + if (tst_fill_file(TEST_FILE, 'b', page_size, 1)) > + tst_brk(TBROK | TERRNO, "fill_file"); > + > + rofd = SAFE_OPEN(TEST_FILE, O_RDONLY | O_CREAT, 0777); > + /* > + * Assuming disk blocks are 8k, and logical pages are 4k, there are > + * two maps per page. In order to test mapping at the beginning and ^ block > + * ends of the block, mapping the whole block, or none of the block > + * with different mappings on preceding and following blocks, each > + * 3 blocks with 6 pages can be thought of as a binary number from 0 to > + * 64 with a bit set for mapped or cleared for unmapped. This number > + * is represented by i. The value j is used to look at the bits of i > + * and decided to map the page or not. > + * NOTE: None of the above assumptions are critical. > + */ > + for (i = 0; i < 64; i++) { > + for (j = 0; j < 6; j++) { > + off_t mapoff; > + > + if (!(i & (1 << j))) > + continue; > + > + mapoff = page_size * (off_t)(6 + i + j); ^ Should be '*'? > + SAFE_MMAP(mmap_area + page_size * (6 * i + j), > + page_size, PROT_READ, > + MAP_FILE | MAP_PRIVATE | MAP_FIXED, > + rofd, mapoff); I wonder if we can just keep a counter for the offset in the mmaped area here, something as: mmapped_pages = 0; SAFE_MMAP(mmap_area + page_size * mapped_pages++, ... Then the loop that checks the data could be just a simple for() loop. > + } > + } > + SAFE_CLOSE(rofd); > + > + /* write out 6 pages of stuff into each of the 64 six page sections */ > + rwfd = SAFE_OPEN(TEST_FILE, O_RDWR); > + buf = SAFE_MALLOC(page_size); > + memset(buf, 'a', page_size); > + for (i = 0; i < 6 * 64; i++) > + write_fully(rwfd, buf, page_size); > + free(buf); > + SAFE_CLOSE(rwfd); > + > + /* > + * Just finished scribbling all over interwoven mmapped and unmapped > + * regions. Check the data. > + */ > + for (i = 0; i < 64; i++) { > + for (j = 0; j < 6; j++) { > + unsigned char val; > + > + if (!(i & (1 << j))) > + continue; > + > + val = *(mmap_area + page_size * (6 * i + j)); > + if (val != 'a') { > + tst_res(TFAIL, "unexpected value in map, " > + "i=%d,j=%d,val=0x%x", i, j, val); > + ret = 1; > + goto done; > + } Maybe we should check whole page, not just first byte. > + } > + } > +done: > + tst_res(ret ? TFAIL : TPASS, "blocks have expected data"); This would produce failing message with 'block have expected data' text if we get to the goto above. Why isn't the done: label pointing just before the SAFE_UNLIK()? It does to make any sense to print the failure message twice here. > + SAFE_UNLINK(TEST_FILE); > +} > + > +static struct tst_test test = { > + .tid = "mmapstress04", > + .needs_tmpdir = 1, > + .test_all = mmapstress04, > + .setup = setup, > +}; > -- > 1.8.3.1 > > > -- > Mailing list info: https://lists.linux.it/listinfo/ltp -- Cyril Hrubis chrubis@suse.cz