Bug 74773

Summary:	Wrong handling of mmap failure
Product:	p11-glue	Reporter:	Pascal Terjan <pterjan>
Component:	p11-kit	Assignee:	Stef Walter <stefw>
Status:	RESOLVED FIXED	QA Contact:
Severity:	normal
Priority:	medium	CC:	gilles.mouchard, stefw
Version:	unspecified
Hardware:	Other
OS:	All
Whiteboard:
i915 platform:		i915 features:
Attachments:	Fix the tested value Add a test Fix handling of mmap failure and mapping empty files

Description Pascal Terjan 2014-02-10 07:10:38 UTC

Created attachment 93744 [details] [review]
Fix the tested value

mmap man page:
On error, the value MAP_FAILED (that is, (void *) -1) is returned, and errno is set appropriately.

Code in p11_mmap_open:
        map->data = mmap (NULL, map->size, PROT_READ, MAP_PRIVATE, map->fd, 0);
        if (map->data == NULL) {
                close (map->fd);
                free (map);
                return NULL;
        }

        *data = map->data;
        *size = map->size;
        return map;

map->data will never be NULL, it will be either an address or -1.

Comment 1 Pascal Terjan 2014-02-10 07:16:17 UTC

Also, to see the problem, you can create an empty conf file as mmap on an empty file fails.

Comment 2 Pascal Terjan 2014-02-11 18:19:18 UTC

Created attachment 93879 [details] [review]
Add a test

Strace of the test:

[pid 13017] open("emptyfileU0ly3U", O_RDWR|O_CREAT|O_EXCL, 0600) = 3
[pid 13017] close(3)                    = 0
[pid 13017] open("emptyfileU0ly3U", O_RDONLY|O_CLOEXEC) = 3
[pid 13017] fstat(3, {st_mode=S_IFREG|0600, st_size=0, ...}) = 0
[pid 13017] mmap(NULL, 0, PROT_READ, MAP_PRIVATE, 3, 0) = -1 EINVAL (Invalid argument)
[pid 13017] close(3)                    = 0
[pid 13017] unlink("emptyfileU0ly3U")   = 0

Comment 3 Gilles Mouchard 2014-02-12 21:17:08 UTC

*** Bug 74904 has been marked as a duplicate of this bug. ***

Comment 4 Stef Walter 2014-02-13 07:25:46 UTC

Created attachment 93989 [details] [review]
Fix handling of mmap failure and mapping empty files

Thanks for the patch and test. I've updated the patch
to include support for mapping empty files. 

https://bugs.freedesktop.org/show_bug.cgi?id=74773

Signed-off-by: Stef Walter <stef@thewalter.net>

Comment 5 Stef Walter 2014-02-13 07:26:52 UTC

Attachment 93989 [details] pushed as 44beedb - Fix handling of mmap failure and mapping empty files

Comment 6 Stef Walter 2014-02-13 07:41:57 UTC

Pushed a similar patch to stable branch.

Comment 7 Pascal Terjan 2014-02-13 09:53:09 UTC

Thanks, even better like that :)

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.