Re: [Qemu-devel] [PATCH] linux-user: add memfd_create

[Laurent Vivier <laurent@vivier.eu>]

Le 19/08/2019 à 13:56, Peter Maydell a écrit :
> On Fri, 16 Aug 2019 at 22:28, Shu-Chun Weng via Qemu-devel
> <qemu-devel@nongnu.org> wrote:
>>
>> Add support for the memfd_create syscall. If the host does not have the
>> libc wrapper, translate to a direct syscall with NC-macro.
>>
>> Buglink: https://bugs.launchpad.net/qemu/+bug/1734792
>> Signed-off-by: Shu-Chun Weng <scw@google.com>
>> ---
>>  include/qemu/memfd.h |  4 ++++
>>  linux-user/syscall.c | 11 +++++++++++
>>  util/memfd.c         |  2 +-
>>  3 files changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/qemu/memfd.h b/include/qemu/memfd.h
>> index d551c28b68..975b6bdb77 100644
>> --- a/include/qemu/memfd.h
>> +++ b/include/qemu/memfd.h
>> @@ -32,6 +32,10 @@
>>  #define MFD_HUGE_SHIFT 26
>>  #endif
>>
>> +#if defined CONFIG_LINUX && !defined CONFIG_MEMFD
>> +int memfd_create(const char *name, unsigned int flags);
>> +#endif
>> +
>>  int qemu_memfd_create(const char *name, size_t size, bool hugetlb,
>>                        uint64_t hugetlbsize, unsigned int seals, Error **errp);
>>  bool qemu_memfd_alloc_check(void);
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 8367cb138d..b506c1f40e 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -20,6 +20,7 @@
>>  #include "qemu/osdep.h"
>>  #include "qemu/cutils.h"
>>  #include "qemu/path.h"
>> +#include "qemu/memfd.h"
>>  #include <elf.h>
>>  #include <endian.h>
>>  #include <grp.h>
>> @@ -11938,6 +11939,16 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
>>          /* PowerPC specific.  */
>>          return do_swapcontext(cpu_env, arg1, arg2, arg3);
>>  #endif
>> +#ifdef TARGET_NR_memfd_create
>> +    case TARGET_NR_memfd_create:
>> +        p = lock_user_string(arg1);
>> +        if (!p) {
>> +            return -TARGET_EFAULT;
>> +        }
>> +        ret = get_errno(memfd_create(p, arg2));
> 
> I think here you may need to call
>            fd_trans_unregister(ret);
> 
> since memfd_create() returns a file descriptor.
> (This call unregisters any pre-existing hooks for "we need
> to mangle the data for reads/writes of this file descriptor",
> in case the fd was previously being used for a file descriptor
> type that needed that.) This is what eg NR_open does.
> 
> Laurent -- do I have this right? It's not entirely clear
> to me that we could ever be in a situation where a syscall
> like open or memfd_create returns an fd that's got an fd_trans
> hook active, because that would imply we'd failed to delete
> the hook on fd-close somehow. Is this just a belt-and-braces
> bit of coding ?

Exactly.

It's in case we missed one fd close and then to be sure an existing "fd
translator" will not mess with data of the new one.

Thanks,
Laurent