[libcamera-devel] [PATCH 1/2] v4l2: v4l2_compat: Intercept open64, openat64, and mmap64

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Tue, Jun 16, 2020 at 07:56:32PM +0900, Paul Elder wrote:
> Some applications (eg. Firefox, Google Chrome, Skype) use import open64,

use or import ?

> openat64, and mmap64 instead of their non-64 versions that we currently
> intercept. Intercept these calls as well.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
>  src/v4l2/meson.build             |  1 +
>  src/v4l2/v4l2_camera_proxy.cpp   |  2 +-
>  src/v4l2/v4l2_camera_proxy.h     |  3 ++-
>  src/v4l2/v4l2_compat.cpp         | 29 ++++++++++++++++++++++++++++-
>  src/v4l2/v4l2_compat_manager.cpp |  2 +-
>  src/v4l2/v4l2_compat_manager.h   |  5 +++--
>  6 files changed, 36 insertions(+), 6 deletions(-)
> 
> diff --git a/src/v4l2/meson.build b/src/v4l2/meson.build
> index 0fb941e..f2e4aaf 100644
> --- a/src/v4l2/meson.build
> +++ b/src/v4l2/meson.build
> @@ -15,6 +15,7 @@ v4l2_compat_cpp_args = [
>      # file operations, disable transparent large file support.
>      '-U_FILE_OFFSET_BITS',
>      '-D_FILE_OFFSET_BITS=32',
> +    '-D_LARGEFILE64_SOURCE',

This deserves an explanation in the commit message.

>      '-fvisibility=hidden',
>  ]
>  
> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
> index 17477ab..59af209 100644
> --- a/src/v4l2/v4l2_camera_proxy.cpp
> +++ b/src/v4l2/v4l2_camera_proxy.cpp
> @@ -75,7 +75,7 @@ void V4L2CameraProxy::close()
>  }
>  
>  void *V4L2CameraProxy::mmap(void *addr, size_t length, int prot, int flags,
> -			    off_t offset)
> +			    __off64_t offset)
>  {
>  	LOG(V4L2Compat, Debug) << "Servicing mmap";
>  
> diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h
> index 7c65c88..b5a9801 100644
> --- a/src/v4l2/v4l2_camera_proxy.h
> +++ b/src/v4l2/v4l2_camera_proxy.h
> @@ -11,6 +11,7 @@
>  #include <linux/videodev2.h>
>  #include <map>
>  #include <memory>
> +#include <sys/mman.h>
>  #include <sys/types.h>
>  #include <vector>
>  
> @@ -28,7 +29,7 @@ public:
>  	int open(bool nonBlocking);
>  	void dup();
>  	void close();
> -	void *mmap(void *addr, size_t length, int prot, int flags, off_t offset);
> +	void *mmap(void *addr, size_t length, int prot, int flags, __off64_t offset);

Can't you use off64_t ? Types starting with a double underscore are
internal to the libc implementation. While this will work with glibc and
uclibc, musl doesn't provide __off64_t, but provides off64_t.

>  	int munmap(void *addr, size_t length);
>  
>  	int ioctl(unsigned long request, void *arg);
> diff --git a/src/v4l2/v4l2_compat.cpp b/src/v4l2/v4l2_compat.cpp
> index 2a9a176..ead36e6 100644
> --- a/src/v4l2/v4l2_compat.cpp
> +++ b/src/v4l2/v4l2_compat.cpp
> @@ -38,6 +38,16 @@ LIBCAMERA_PUBLIC int open(const char *path, int oflag, ...)
>  						     oflag, mode);
>  }
>  
> +LIBCAMERA_PUBLIC int open64(const char *path, int oflag, ...)
> +{
> +	mode_t mode = 0;
> +	if (oflag & O_CREAT || oflag & O_TMPFILE)
> +		extract_va_arg(mode_t, mode, oflag);
> +
> +	return V4L2CompatManager::instance()->openat(AT_FDCWD, path,
> +						     oflag | O_LARGEFILE, mode);
> +}
> +
>  /* _FORTIFY_SOURCE redirects open to __open_2 */
>  LIBCAMERA_PUBLIC int __open_2(const char *path, int oflag)
>  {
> @@ -53,6 +63,16 @@ LIBCAMERA_PUBLIC int openat(int dirfd, const char *path, int oflag, ...)
>  	return V4L2CompatManager::instance()->openat(dirfd, path, oflag, mode);
>  }
>  
> +LIBCAMERA_PUBLIC int openat64(int dirfd, const char *path, int oflag, ...)
> +{
> +	mode_t mode = 0;
> +	if (oflag & O_CREAT || oflag & O_TMPFILE)
> +		extract_va_arg(mode_t, mode, oflag);
> +
> +	return V4L2CompatManager::instance()->openat(dirfd, path,
> +						     oflag | O_LARGEFILE, mode);
> +}
> +
>  LIBCAMERA_PUBLIC int __openat_2(int dirfd, const char *path, int oflag)
>  {
>  	return openat(dirfd, path, oflag);
> @@ -69,7 +89,14 @@ LIBCAMERA_PUBLIC int close(int fd)
>  }
>  
>  LIBCAMERA_PUBLIC void *mmap(void *addr, size_t length, int prot, int flags,
> -			    int fd, off_t offset)
> +			    int fd, __off_t offset)
> +{
> +	return V4L2CompatManager::instance()->mmap(addr, length, prot, flags,
> +						   fd, offset);
> +}
> +
> +LIBCAMERA_PUBLIC void *mmap64(void *addr, size_t length, int prot, int flags,
> +			      int fd, __off64_t offset)

With musl, when _LARGEFILE64_SOURCE is define, mmap64 is handled with

#define mmap64 mmap
#define off64_t off_t

Other functions are handled in a similar way. You will thus have an
issue here.

There doesn't seem to be an easy way to detect musl (as in #ifdef
__MUSL__ for instance), but I think we can address this with

#ifndef mmap64
LIBCAMERA_PUBLIC void *mmap64(void *addr, size_t length, int prot, int flags,
			      int fd, __off64_t offset)
{
...
}
#endif

as with glibc and uclibc mmap64 is a function, not a macro. Same for the
other functions.

>  {
>  	return V4L2CompatManager::instance()->mmap(addr, length, prot, flags,
>  						   fd, offset);
> diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp
> index 56533c4..f928760 100644
> --- a/src/v4l2/v4l2_compat_manager.cpp
> +++ b/src/v4l2/v4l2_compat_manager.cpp
> @@ -200,7 +200,7 @@ int V4L2CompatManager::close(int fd)
>  }
>  
>  void *V4L2CompatManager::mmap(void *addr, size_t length, int prot, int flags,
> -			      int fd, off_t offset)
> +			      int fd, __off64_t offset)
>  {
>  	V4L2CameraProxy *proxy = getProxy(fd);
>  	if (!proxy)
> diff --git a/src/v4l2/v4l2_compat_manager.h b/src/v4l2/v4l2_compat_manager.h
> index 14338a5..baf23ae 100644
> --- a/src/v4l2/v4l2_compat_manager.h
> +++ b/src/v4l2/v4l2_compat_manager.h
> @@ -11,6 +11,7 @@
>  #include <fcntl.h>
>  #include <map>
>  #include <memory>
> +#include <sys/mman.h>
>  #include <sys/types.h>
>  #include <vector>
>  
> @@ -30,7 +31,7 @@ public:
>  		using close_func_t = int (*)(int fd);
>  		using ioctl_func_t = int (*)(int fd, unsigned long request, ...);
>  		using mmap_func_t = void *(*)(void *addr, size_t length, int prot,
> -					      int flags, int fd, off_t offset);
> +					      int flags, int fd, __off64_t offset);
>  		using munmap_func_t = int (*)(void *addr, size_t length);

The redirection is performed with

        get_symbol(fops_.openat, "openat");
        get_symbol(fops_.dup, "dup");
        get_symbol(fops_.close, "close");
        get_symbol(fops_.ioctl, "ioctl");
        get_symbol(fops_.mmap, "mmap");
        get_symbol(fops_.munmap, "munmap");

Don't you thus redirect to 32-bit versions ? Shouldn't this become
openat64 and mmap64 ?

The uclibc implementation is very similar to glibc, but musl is quite
different. I would thus recommend at least compile-testing the series
against musl. You can build a minimal musl-based rootfs with buildroot
and use that to "cross-compile" libcamera (not sure if compiling for x86
on x86 technically counts as cross-compilation :-)) for musl.

>  
>  		openat_func_t openat;
> @@ -51,7 +52,7 @@ public:
>  	int dup(int oldfd);
>  	int close(int fd);
>  	void *mmap(void *addr, size_t length, int prot, int flags,
> -		   int fd, off_t offset);
> +		   int fd, __off64_t offset);
>  	int munmap(void *addr, size_t length);
>  	int ioctl(int fd, unsigned long request, void *arg);
>  

-- 
Regards,

Laurent Pinchart