Page MenuHomeFreeBSD

Refinement of extended errors for mmap(2)
ClosedPublic

Authored by mckusick on Tue, Mar 17, 12:50 AM.

Details

Summary

Provide more precise error explanations for several of the mmap(2) EINVAL errors.

Test Plan

Run this program to verify that the updates are sensible:

#include <err.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <sys/file.h>
#include <sys/mman.h>
#include <sys/stat.h>

int
main(int ac, char *av[])
{
	struct stat st;
	size_t len;
	off_t offset;
	int fd;
	char *mapaddr;

	if (ac != 2) {
		printf("Usage: %s file_to_map\n", av[0]);
		exit(1);
	}
	if ((fd = open(av[1], O_CREAT|O_RDWR, 0664)) < 0) {
		err(2, "open %s failed", av[1]);
	}
	if ((ftruncate(fd, 0x6400000)) < 0) {
		err(3, "truncate %s failed", av[1]);
	}
	if ((fstat(fd, &st)) < 0) {
		err(4, "stat %s failed", av[1]);
	}
	len = st.st_size;
	printf("%s size = %ld\n", av[1], len);
	offset = 0;
	if ((mapaddr = mmap(0, len, MAP_NOSYNC, PROT_READ, fd, offset)) == (char *)-1) {
		warn("mmap %s failed1", av[1]);
	}
	if ((mapaddr = mmap(0, 0, PROT_READ, MAP_ANON, fd, offset)) == (char *)-1) {
		warn("mmap %s failed2", av[1]);
	}
	if ((mapaddr = mmap(0, len, PROT_READ, MAP_STACK, fd, offset)) == (char *)-1) {
		warn("mmap %s failed3", av[1]);
	}
	if ((mapaddr = mmap(0, len, PROT_READ, MAP_RESERVED0080, fd, offset)) == (char *)-1) {
		warn("mmap %s failed4", av[1]);
	}
	if ((mapaddr = mmap(0, len, PROT_READ, MAP_EXCL, fd, offset)) == (char *)-1) {
		warn("mmap %s failed5", av[1]);
	}
	if ((mapaddr = mmap(0, len, PROT_READ, MAP_SHARED | MAP_PRIVATE, fd, offset)) == (char *)-1) {
		warn("mmap %s failed6", av[1]);
	}
	if ((mapaddr = mmap(0, len, PROT_READ, MAP_GUARD, fd, offset)) == (char *)-1) {
		warn("mmap %s failed7", av[1]);
	}
	if ((mapaddr = mmap(0, len, PROT_READ, MAP_ALIGNED(2), fd, offset)) == (char *)-1) {
		warn("mmap %s failed8", av[1]);
	}
	if ((mapaddr = mmap((char *)1, len, PROT_READ, MAP_FIXED, fd, offset)) == (char *)-1) {
		warn("mmap %s failed9", av[1]);
	}
	if ((mapaddr = mmap((void *)0x0000000080000000, len, PROT_READ, MAP_FIXED, fd, offset)) == (char *)-1) {
		warn("mmap %s failed10", av[1]);
	}
	if ((mapaddr = mmap((void *)0x0000000080000000, len, PROT_READ, MAP_32BIT|MAP_FIXED, fd, offset)) == (char *)-1) {
		warn("mmap %s failed11", av[1]);
	}
	printf("Mapped at 0x%lx\n", (unsigned long)mapaddr);
	close(fd);
	exit(0);
}

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

sys/vm/vm_mmap.c
239–245

If doing that, I suggest not to repeat the tests. Split the main error check. as you did in the next chunk.

Update per kib's suggestion.

Update per your suggestion.

sys/vm/vm_mmap.c
239–245

Isn't the new version changes the logic?

Before, if len == 0 and p_osrel >= something, we returned EINVAL.
Now it would only do that if fd != -1 or pos != 0.

I believe more splitting of the if() conditions is needed. And each case would get its own exterr.

Valid complaint, but not sure what the error message should be for the third case.

sys/vm/vm_mmap.c
239–245

You are right that we did return EINVAL, but with an error that complained about two things that were not wrong. What should be the error message if (len == 0 && p->p_osrel >= P_OSREL_MAP_ANON)?

sys/vm/vm_mmap.c
239–245

I propose "mapping with zero length".

mckusick edited the test plan for this revision. (Show Details)

Further error message refinement.

This time for sure!

kib added inline comments.
sys/vm/vm_mmap.c
277

This change should be a separate commit with the proper commit message.

329
This revision is now accepted and ready to land.Wed, Mar 18, 12:56 AM