Re: [PATCH v6 3/7] um: use execveat to create userspace MMs

[Johannes Berg <johannes@sipsolutions.net>]

On Wed, 2024-06-26 at 15:53 +0200, Benjamin Berg wrote:
> 
> +static int __init init_stub_exec_fd(void)
> +{
> +	size_t len = 0;
> +	int res;
> +	char tmpfile[] = "/tmp/uml-userspace-XXXXXX";


That seems awkward, perhaps it should use make_tempfile() from mem.c?

> +		stub_exec_fd = mkostemp(tmpfile, O_CLOEXEC);

mkostemp() also requires _GNU_SOURCE according to the man page? It also
doesn't matter since you reopen the file anyway.

> +		/* Only executable by us */
> +		if (fchmod(stub_exec_fd, 00100) < 0) {
> +			unlink(tmpfile);
> +			panic("Could not make stub binary excutable: %d",
> +			      errno);
> +		}
> +
> +		close(stub_exec_fd);
> +		stub_exec_fd = open(tmpfile, O_CLOEXEC);

Hmm. Technically, I think you _have_ to open for reading, writing, or
both; not none? But then I guess you have to make it readable?

Might also want O_NOFOLLOW here?

> diff --git a/arch/x86/um/stub_elf.c b/arch/x86/um/stub_elf.c
> new file mode 100644
> index 000000000000..2bf1a717065d
> --- /dev/null
> +++ b/arch/x86/um/stub_elf.c

Is stub_elf really the right name? In practice it's going to be an ELF
file, but ... who cares? Not sure why it should be called "elf" vs. just
"stub" or "exec_stub" or something like that.

Also, is it really x86-specific?

> +++ b/arch/x86/um/stub_elf_embed.S

surely that isn't x86-specific?

johannes