linux-kernel - Re: [PATCHv6 1/3] syscalls,x86: implement execveat() system call

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHse=S9sudiJnLTwd1yXK9jFCPfVq4uPLBjhUDqMFeh2UQ7xqQ@mail.gmail.com>
Date:	Fri, 7 Nov 2014 13:19:54 +0000
From:	David Drysdale <drysdale@...gle.com>
To:	Kees Cook <keescook@...omium.org>
Cc:	"Eric W. Biederman" <ebiederm@...ssion.com>,
	Andy Lutomirski <luto@...capital.net>,
	Alexander Viro <viro@...iv.linux.org.uk>,
	Meredydd Luff <meredydd@...atehouse.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Arnd Bergmann <arnd@...db.de>, Rich Felker <dalias@...ifal.cx>,
	Christoph Hellwig <hch@...radead.org>,
	"x86@...nel.org" <x86@...nel.org>,
	linux-arch <linux-arch@...r.kernel.org>,
	Linux API <linux-api@...r.kernel.org>
Subject: Re: [PATCHv6 1/3] syscalls,x86: implement execveat() system call

On Thu, Nov 6, 2014 at 6:06 PM, Kees Cook <keescook@...omium.org> wrote:
> On Thu, Nov 6, 2014 at 8:07 AM, David Drysdale <drysdale@...gle.com> wrote:
>> diff --git a/fs/exec.c b/fs/exec.c
>> index a2b42a98c743..800d232c17bb 100644
>> --- a/fs/exec.c
>> +++ b/fs/exec.c
>> @@ -1422,10 +1431,12 @@ static int exec_binprm(struct linux_binprm *bprm)
>>  /*
>>   * sys_execve() executes a new program.
>>   */
>> -static int do_execve_common(struct filename *filename,
>> -                               struct user_arg_ptr argv,
>> -                               struct user_arg_ptr envp)
>> +static int do_execveat_common(int fd, struct filename *filename,
>> +                             struct user_arg_ptr argv,
>> +                             struct user_arg_ptr envp,
>> +                             int flags)
>>  {
>> +       char *pathbuf = NULL;
>>         struct linux_binprm *bprm;
>>         struct file *file;
>>         struct files_struct *displaced;
>> @@ -1466,7 +1477,7 @@ static int do_execve_common(struct filename *filename,
>>         check_unsafe_exec(bprm);
>>         current->in_execve = 1;
>>
>> -       file = do_open_exec(filename);
>> +       file = do_open_execat(fd, filename, flags);
>>         retval = PTR_ERR(file);
>>         if (IS_ERR(file))
>>                 goto out_unmark;
>> @@ -1474,7 +1485,30 @@ static int do_execve_common(struct filename *filename,
>>         sched_exec();
>>
>>         bprm->file = file;
>> -       bprm->filename = bprm->interp = filename->name;
>> +       if (fd == AT_FDCWD || filename->name[0] == '/') {
>> +               bprm->filename = filename->name;
>> +       } else {
>> +               /* "/dev/fd/2147483647/" + filename->name */
>> +               int maxlen = 19 + strlen(filename->name);
>
> This should be 20 + strlen (to include the trailing NULL). However, I
> think this whole bit of code could be replaced with kasprintf...
>
>> +
>> +               pathbuf = kmalloc(maxlen, GFP_TEMPORARY);
>> +               if (!pathbuf) {
>> +                       retval = -ENOMEM;
>> +                       goto out_unmark;
>> +               }
>> +               if (filename->name[0] == '\0')
>> +                       sprintf(pathbuf, "/dev/fd/%d", fd);
>> +               else
>> +                       snprintf(pathbuf, maxlen,
>> +                                "/dev/fd/%d/%s", fd, filename->name);
>
> Maybe something like this? A bit messy, so maybe your original if/else
> would be more readable.
>
>     } else {
>         pathbuf = kasprintf("/dev/fd/%d%s%s", fd,
>                                        filename->name[0] == '\0' ? "" : "/",
>                                        filename->name[0] == '\0' ? ""
> : filename->name);
>         if (!pathbuf) {
>             retval = -ENOMEM;
>             goto out_unmark;
>         }
>     }

Ah, I didn't know about kasprintf(), that will make things much
easier -- thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/