Introduce xcb_aux_alloc_shm() to create anonymous files in RAM

Submitted by Alexander Volkov on April 10, 2018, 11:38 a.m.

Details

Message ID 20180410113805.28780-1-a.volkov@rusbitech.ru
State New
Series "Introduce xcb_aux_alloc_shm() to create anonymous files in RAM"
Headers show

Commit Message

Alexander Volkov April 10, 2018, 11:38 a.m.
... which then can be mapped with mmap().
It is intended to be used in conjunction with xcb_shm_attach_fd()
to access the created shared memory from a client and the X server.

Signed-off-by: Alexander Volkov <a.volkov@rusbitech.ru>
---
 configure.ac  | 47 +++++++++++++++++++++++++++++
 src/xcb_aux.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++
 src/xcb_aux.h |  3 ++
 3 files changed, 133 insertions(+)

Patch hide | download patch | download mbox

diff --git a/configure.ac b/configure.ac
index 1fe1561..81e1f6b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -7,10 +7,57 @@  AC_CONFIG_HEADERS([config.h])
 AC_CONFIG_MACRO_DIR([m4])
 AM_INIT_AUTOMAKE([foreign dist-bzip2])
 
+AC_USE_SYSTEM_EXTENSIONS
+
 XCB_UTIL_COMMON([1.4], [1.6])
 
 AC_CHECK_FUNCS_ONCE(vasprintf)
 
+AC_CHECK_FUNCS(memfd_create mkostemp)
+
+AC_CHECK_DECLS([__NR_memfd_create], [], [], [[#include <asm/unistd.h>]])
+
+AC_CHECK_HEADERS([sys/memfd.h], [AC_DEFINE([HAVE_MEMFD_H], 1, [Has sys/memfd.h header])])
+
+AC_ARG_WITH(shared-memory-dir, AS_HELP_STRING([--with-shared-memory-dir=PATH], [Path to directory in a world-writable temporary directory for anonymous shared memory (default: auto)]),
+[],
+[with_shared_memory_dir=yes])
+
+shmdirs="/run/shm /dev/shm /var/tmp /tmp"
+
+case x"$with_shared_memory_dir" in
+xyes)
+	for dir in $shmdirs; do
+		case x"$with_shared_memory_dir" in
+		xyes)
+			echo Checking temp dir "$dir"
+			if test -d "$dir"; then
+				with_shared_memory_dir="$dir"
+			fi
+			;;
+		esac
+	done
+	;;
+x/*)
+	;;
+xno)
+	;;
+*)
+	AC_MSG_ERROR([Invalid directory specified for --with-shared-memory-dir: $with_shared_memory_dir])
+	;;
+esac
+
+case x"$with_shared_memory_dir" in
+xyes)
+	AC_MSG_ERROR([No directory found for shared memory temp files.])
+	;;
+xno)
+	;;
+*)
+	AC_DEFINE_UNQUOTED(SHMDIR, ["$with_shared_memory_dir"], [Directory for shared memory temp files])
+	;;
+esac
+
 AC_CONFIG_FILES([Makefile
 		src/Makefile
 		xcb-atom.pc
diff --git a/src/xcb_aux.c b/src/xcb_aux.c
index b6f64f8..e8f7ba9 100644
--- a/src/xcb_aux.c
+++ b/src/xcb_aux.c
@@ -33,9 +33,39 @@ 
 #include "config.h"
 #endif
 
+#if !HAVE_MEMFD_CREATE
+#if HAVE_DECL___NR_MEMFD_CREATE
+#include <unistd.h>
+#include <sys/syscall.h>
+static int memfd_create(const char *name,
+			    unsigned int flags)
+{
+	return syscall(__NR_memfd_create, name, flags);
+}
+#define HAVE_MEMFD_CREATE	1
+#endif
+#endif
+
+#if HAVE_MEMFD_CREATE
+
+/* Get defines for the memfd_create syscall, using the
+ * header if available, or just defining the constants otherwise
+ */
+
+#if HAVE_MEMFD_H
+#include <sys/memfd.h>
+#else
+/* flags for memfd_create(2) (unsigned int) */
+#define MFD_CLOEXEC		0x0001U
+#define MFD_ALLOW_SEALING	0x0002U
+#endif
+
+#endif
+
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <fcntl.h>
 
 #include <xcb/xcb.h>
 #include "xcb_aux.h"
@@ -376,3 +406,56 @@  xcb_aux_clear_window(xcb_connection_t *  dpy,
 {
     return xcb_clear_area(dpy, 0, w, 0, 0, 0, 0);
 }
+
+/* SHM related functions */
+
+/**
+ * xcb_aux_alloc_shm:
+ *
+ * Creates an anonymous file of the required size in RAM.
+ * This function is intended for use in conjunction with
+ * xcb_shm_attach_fd().
+ *
+ * Return value: the file descriptor, or -1 on failure
+ * (in which case, errno will be set as appropriate).
+ **/
+int
+xcb_aux_alloc_shm(size_t size)
+{
+	char	template[] = SHMDIR "/shmfd-XXXXXX";
+	int	fd;
+
+#if HAVE_MEMFD_CREATE
+	fd = memfd_create("xcb_aux", MFD_CLOEXEC|MFD_ALLOW_SEALING);
+	if (fd < 0)
+#endif
+	{
+#ifdef O_TMPFILE
+		fd = open(SHMDIR, O_TMPFILE|O_RDWR|O_CLOEXEC|O_EXCL, 0666);
+		if (fd < 0)
+#endif
+		{
+#ifndef HAVE_MKOSTEMP
+			int flags;
+			fd = mkstemp(template);
+#else
+			fd = mkostemp(template, O_CLOEXEC);
+#endif
+			if (fd < 0)
+				return fd;
+			unlink(template);
+#ifndef HAVE_MKOSTEMP
+			flags = fcntl(fd, F_GETFD);
+			if (flags != -1) {
+				flags |= FD_CLOEXEC;
+				(void) fcntl(fd, F_SETFD, &flags);
+			}
+#endif
+		}
+	}
+	if (ftruncate(fd, size) < 0) {
+		close(fd);
+		return -1;
+	}
+	return fd;
+}
diff --git a/src/xcb_aux.h b/src/xcb_aux.h
index da4fb85..75b6e66 100644
--- a/src/xcb_aux.h
+++ b/src/xcb_aux.h
@@ -206,6 +206,9 @@  xcb_void_cookie_t
 xcb_aux_clear_window(xcb_connection_t *  dpy,
 		     xcb_window_t        w);
 
+int
+xcb_aux_alloc_shm(size_t size);
+
 #ifdef __cplusplus
 }
 #endif

Comments

Uli Schlachter April 13, 2018, 6:47 a.m.
On 10.04.2018 13:38, Alexander Volkov wrote:
> ... which then can be mapped with mmap().
> It is intended to be used in conjunction with xcb_shm_attach_fd()
> to access the created shared memory from a client and the X server.
> 
> Signed-off-by: Alexander Volkov <a.volkov@rusbitech.ru>
> ---
>  configure.ac  | 47 +++++++++++++++++++++++++++++
>  src/xcb_aux.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/xcb_aux.h |  3 ++
>  3 files changed, 133 insertions(+)
> 
> diff --git a/configure.ac b/configure.ac
> index 1fe1561..81e1f6b 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -7,10 +7,57 @@ AC_CONFIG_HEADERS([config.h])
>  AC_CONFIG_MACRO_DIR([m4])
>  AM_INIT_AUTOMAKE([foreign dist-bzip2])
>  
> +AC_USE_SYSTEM_EXTENSIONS
> +
>  XCB_UTIL_COMMON([1.4], [1.6])
>  
>  AC_CHECK_FUNCS_ONCE(vasprintf)
>  
> +AC_CHECK_FUNCS(memfd_create mkostemp)
> +
> +AC_CHECK_DECLS([__NR_memfd_create], [], [], [[#include <asm/unistd.h>]])
> +
> +AC_CHECK_HEADERS([sys/memfd.h], [AC_DEFINE([HAVE_MEMFD_H], 1, [Has sys/memfd.h header])])
> +
> +AC_ARG_WITH(shared-memory-dir, AS_HELP_STRING([--with-shared-memory-dir=PATH], [Path to directory in a world-writable temporary directory for anonymous shared memory (default: auto)]),

Default is yes, not auto. See next two lines.

> +[],
> +[with_shared_memory_dir=yes])
> +
> +shmdirs="/run/shm /dev/shm /var/tmp /tmp"
> +
> +case x"$with_shared_memory_dir" in
> +xyes)
> +	for dir in $shmdirs; do
> +		case x"$with_shared_memory_dir" in
> +		xyes)

How many shells are there that do not like "break"? :-/
(And does autoconf provide some way to make this loop nicer?)

> +			echo Checking temp dir "$dir"
> +			if test -d "$dir"; then
> +				with_shared_memory_dir="$dir"
> +			fi
> +			;;
> +		esac
> +	done
> +	;;
> +x/*)
> +	;;
> +xno)
> +	;;
> +*)
> +	AC_MSG_ERROR([Invalid directory specified for --with-shared-memory-dir: $with_shared_memory_dir])
> +	;;
> +esac
> +
> +case x"$with_shared_memory_dir" in
> +xyes)
> +	AC_MSG_ERROR([No directory found for shared memory temp files.])
> +	;;
> +xno)
> +	;;
> +*)
> +	AC_DEFINE_UNQUOTED(SHMDIR, ["$with_shared_memory_dir"], [Directory for shared memory temp files])
> +	;;
> +esac
> +
>  AC_CONFIG_FILES([Makefile
>  		src/Makefile
>  		xcb-atom.pc

So, if I do --with-shared-memory-dir=/foobar, configure will tell me
that I specified an invalid directory? Why?? And why does this flag
exist at all, i.e. what would be uses for it?

Also, why does the existence of a directory at compile time have any
significance at run time? What about cross compiling? Building a distro
package with /run/shm existing and then running it on another system
that does not have /run/shm?

I really think that this compile time check is plain wrong.

Also, is it really worth it to have all these different cases?

- Linux with updated glibc that provides a memfd_create() wrapper
- Linux with older glibc where this code provides its own wrapper
- Older Linux with memfd_create() (-> mkostemp())
- Anything else (-> mkstemp())

How about dropping the second one (own syscall wrapper if glibc does not
provide one). Oh and how about making sys/memfd.h required instead of
providing the defines here instead, if needed.

What would be the downside of this? How many people would complain?

> diff --git a/src/xcb_aux.c b/src/xcb_aux.c
> index b6f64f8..e8f7ba9 100644
> --- a/src/xcb_aux.c
> +++ b/src/xcb_aux.c
> @@ -33,9 +33,39 @@
>  #include "config.h"
>  #endif
>  
> +#if !HAVE_MEMFD_CREATE
> +#if HAVE_DECL___NR_MEMFD_CREATE
> +#include <unistd.h>
> +#include <sys/syscall.h>
> +static int memfd_create(const char *name,
> +			    unsigned int flags)
> +{
> +	return syscall(__NR_memfd_create, name, flags);
> +}
> +#define HAVE_MEMFD_CREATE	1
> +#endif
> +#endif
> +
> +#if HAVE_MEMFD_CREATE
> +
> +/* Get defines for the memfd_create syscall, using the
> + * header if available, or just defining the constants otherwise
> + */
> +
> +#if HAVE_MEMFD_H
> +#include <sys/memfd.h>
> +#else
> +/* flags for memfd_create(2) (unsigned int) */
> +#define MFD_CLOEXEC		0x0001U
> +#define MFD_ALLOW_SEALING	0x0002U
> +#endif

This looks quite linux-specific. Is it worth adding a linux-specific #if
in here in case any other platform ever implements this API as well and
assigns flags differently?

Also, why MFD_ALLOW_SEALING?

[...]
> +int
> +xcb_aux_alloc_shm(size_t size)

Should the documentation mention anything about sealing? close-on-exec?

> +{
> +	char	template[] = SHMDIR "/shmfd-XXXXXX";

This uses shmfd, but...

> +	int	fd;
> +
> +#if HAVE_MEMFD_CREATE
> +	fd = memfd_create("xcb_aux", MFD_CLOEXEC|MFD_ALLOW_SEALING);
[...]

...this uses xcb_aux.

Shouldn't they best use the same template name?

Cheers,
Uli
Alexander Volkov April 17, 2018, 10:52 a.m.
13.04.2018 09:47, Uli Schlachter пишет:
>> +[],
>> +[with_shared_memory_dir=yes])
>> +
>> +shmdirs="/run/shm /dev/shm /var/tmp /tmp"
>> +
>> +case x"$with_shared_memory_dir" in
>> +xyes)
>> +	for dir in $shmdirs; do
>> +		case x"$with_shared_memory_dir" in
>> +		xyes)
> How many shells are there that do not like "break"? :-/
> (And does autoconf provide some way to make this loop nicer?)

> So, if I do --with-shared-memory-dir=/foobar, configure will tell me
> that I specified an invalid directory? Why?? And why does this flag
> exist at all, i.e. what would be uses for it?
>
> Also, why does the existence of a directory at compile time have any
> significance at run time? What about cross compiling? Building a distro
> package with /run/shm existing and then running it on another system
> that does not have /run/shm?
>
> I really think that this compile time check is plain wrong.

Well, I just copied it from 
https://cgit.freedesktop.org/xorg/lib/libxshmfence/tree/configure.ac
The same code is used in the X server: 
https://cgit.freedesktop.org/xorg/xserver/tree/configure.ac#n1121
Somehow it's working there...

> Also, is it really worth it to have all these different cases?
>
> - Linux with updated glibc that provides a memfd_create() wrapper
> - Linux with older glibc where this code provides its own wrapper
> - Older Linux with memfd_create() (-> mkostemp())
> - Anything else (-> mkstemp())
>
> How about dropping the second one (own syscall wrapper if glibc does not
> provide one). Oh and how about making sys/memfd.h required instead of
> providing the defines here instead, if needed.
>
> What would be the downside of this? How many people would complain?
memfd_create() was introduced only in the latest glibc 2.27,
there are many systems where it's not available yet.
I'd prefer not to drop the wrapper.

>> +
>> +#if HAVE_MEMFD_CREATE
>> +
>> +/* Get defines for the memfd_create syscall, using the
>> + * header if available, or just defining the constants otherwise
>> + */
>> +
>> +#if HAVE_MEMFD_H
>> +#include <sys/memfd.h>
>> +#else
>> +/* flags for memfd_create(2) (unsigned int) */
>> +#define MFD_CLOEXEC		0x0001U
>> +#define MFD_ALLOW_SEALING	0x0002U
>> +#endif
> This looks quite linux-specific. Is it worth adding a linux-specific #if
> in here in case any other platform ever implements this API as well and
> assigns flags differently?
I copied this from 
https://cgit.freedesktop.org/xorg/lib/libxshmfence/tree/src/xshmfence_alloc.c
Yes, it seems to be better to make sys/memfd.h required.

> Also, why MFD_ALLOW_SEALING?
>
> [...]
>> +int
>> +xcb_aux_alloc_shm(size_t size)
> Should the documentation mention anything about sealing? close-on-exec?
Yes, it is worth mentioning them.
Besides the size of the file should be fixed with F_ADD_SEALS.

>> +{
>> +	char	template[] = SHMDIR "/shmfd-XXXXXX";
> This uses shmfd, but...
>
>> +	int	fd;
>> +
>> +#if HAVE_MEMFD_CREATE
>> +	fd = memfd_create("xcb_aux", MFD_CLOEXEC|MFD_ALLOW_SEALING);
> [...]
>
> ...this uses xcb_aux.
>
> Shouldn't they best use the same template name?
Agree.
Julien Cristau April 17, 2018, 1:26 p.m.
On 04/17/2018 12:52 PM, Alexander Volkov wrote:
> 13.04.2018 09:47, Uli Schlachter пишет:
>>> +[],
>>> +[with_shared_memory_dir=yes])
>>> +
>>> +shmdirs="/run/shm /dev/shm /var/tmp /tmp"
>>> +
>>> +case x"$with_shared_memory_dir" in
>>> +xyes)
>>> +    for dir in $shmdirs; do
>>> +        case x"$with_shared_memory_dir" in
>>> +        xyes)
>> How many shells are there that do not like "break"? :-/
>> (And does autoconf provide some way to make this loop nicer?)
> 
>> So, if I do --with-shared-memory-dir=/foobar, configure will tell me
>> that I specified an invalid directory? Why?? And why does this flag
>> exist at all, i.e. what would be uses for it?
>>
>> Also, why does the existence of a directory at compile time have any
>> significance at run time? What about cross compiling? Building a distro
>> package with /run/shm existing and then running it on another system
>> that does not have /run/shm?
>>
>> I really think that this compile time check is plain wrong.
> 
> Well, I just copied it from 
> https://cgit.freedesktop.org/xorg/lib/libxshmfence/tree/configure.ac
> The same code is used in the X server: 
> https://cgit.freedesktop.org/xorg/xserver/tree/configure.ac#n1121
> Somehow it's working there...
> 
What's the use of replicating the code from libxshmfence instead of just 
calling into that library?  Or are they somehow subtly different (and if 
so why)?

Cheers,
Julien
Alexander Volkov April 17, 2018, 2:32 p.m.
17.04.2018 16:26, Julien Cristau пишет:
> What's the use of replicating the code from libxshmfence instead of 
> just calling into that library?  Or are they somehow subtly different 
> (and if so why)?
See 
https://cgit.freedesktop.org/xorg/lib/libxshmfence/tree/src/xshmfence_alloc.c
xshmfence_alloc_shm() allocated memory and initializes it for xshmfence 
structure,
while introduced xcb_aux_alloc_shm() is a general-purpose function that 
allocates
memory segment of a specified size.