On Fri, Jan 06, 2017 at 09:26:00PM +0100, Roman Pen wrote: > While doing 'insert range' start block should be also shifted right. > The bug can be easily reproduced by the following test: > > ptr = malloc(4096); > assert(ptr); > > fd = open("./ext4.file", O_CREAT | O_TRUNC | O_RDWR, 0600); > assert(fd >= 0); > > rc = fallocate(fd, 0, 0, 8192); > assert(rc == 0); > for (i = 0; i < 2048; i++) > *((unsigned short *)ptr + i) = 0xbeef; > rc = pwrite(fd, ptr, 4096, 0); > assert(rc == 4096); > rc = pwrite(fd, ptr, 4096, 4096); > assert(rc == 4096); > > for (block = 2; block < 1000; block++) { > rc = fallocate(fd, FALLOC_FL_INSERT_RANGE, 4096, 4096); > assert(rc == 0); > > for (i = 0; i < 2048; i++) > *((unsigned short *)ptr + i) = block; > > rc = pwrite(fd, ptr, 4096, 4096); > assert(rc == 4096); > } > > Because start block is not included in the range the hole appears at > the wrong offset (just after the desired offset) and the following > pwrite() overwrites already existent block, keeping hole untouched. > > Simple way to verify wrong behaviour is to check zeroed blocks after > the test: > > $ hexdump ./ext4.file | grep '0000 0000' > > The root cause of the bug is a wrong range (start, stop], where start > should be inclusive, i.e. [start, stop]. > > This patch fixes the problem by including start into the range. But > not to break left shift (range collapse) stop points to the beginning > of the a block, not to the end. > > The other not obvious change is an iterator check on validness in a > main loop. Because iterator is unsigned the following corner case > should be considered with care: insert a block at 0 offset, when stop > variables overflows and never becomes less than start, which is 0. > To handle this special case iterator is set to NULL to indicate that > end of the loop is reached. > > Signed-off-by: Roman Pen <roman.penyaev@xxxxxxxxxxxxxxxx> Thanks, applied. - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html