All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Ren Zhijie <renzhijie2@huawei.com>,
	viro@zeniv.linux.org.uk, linux-mm@kvack.org,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	tanghui20@huawei.com
Subject: Re: [PATCH] exec: Force binary name when argv is empty
Date: Tue, 20 Sep 2022 15:21:11 -0700	[thread overview]
Message-ID: <202209201516.966D6EF@keescook> (raw)
In-Reply-To: <87sfkmyumv.fsf@email.froward.int.ebiederm.org>

On Tue, Sep 20, 2022 at 09:42:48AM -0500, Eric W. Biederman wrote:
> Ren Zhijie <renzhijie2@huawei.com> writes:
> > From: Hui Tang <tanghui20@huawei.com>
> >
> > First run './execv-main execv-child', there is empty in 'COMMAND' column
> > when run 'ps -u'.
> >
> >  USER       PID %CPU %MEM    VSZ   RSS TTY    [...] TIME COMMAND
> >  root       368  0.3  0.0   4388   764 ttyS0        0:00 ./execv-main
> >  root       369  0.6  0.0   4520   812 ttyS0        0:00
> >
> > The program 'execv-main' as follows:
> >
> >  int main(int argc, char **argv)
> >  {
> >    char *execv_argv[] = {NULL};
> >    pid_t pid = fork();
> >
> >    if (pid == 0) {
> >      execv(argv[1], execv_argv);
> >    } else if (pid > 0) {
> >      wait(NULL);
> >    }
> >    return 0;
> >  }

The correct fix is to userspace here:

  int main(int argc, char **argv)
  {
-   char *execv_argv[] = {NULL};
+   char *execv_argv[] = { argv[1], NULL };
    pid_t pid = fork();

    if (pid == 0) {

> [...]
> For a rare case that should essentially never happen why make it
> friendlier to use?  Why not fix userspace to add the friendly name
> instead of the kernel?
> 
> Unless there is a good reason for it, it would be my hope that in
> a couple of years all of the userspace programs that trigger
> the warning when they start up could be fixed, and we could have
> execve start failing in those cases.

Agreed -- the goal is to help userspace fix how execve(2) is called.

Speaking to the proposed patch, this idea was considered during the
development of the ""-adding patch, with the basic outcome being
that creating a _new_ behavior was not a good idea, and might cause more
confusion. You can see the thread here:

https://lore.kernel.org/all/202202021229.9681AD39B0@keescook/

-Kees

-- 
Kees Cook

  reply	other threads:[~2022-09-20 22:21 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-20 12:08 [PATCH] exec: Force binary name when argv is empty Ren Zhijie
2022-09-20 14:42 ` Eric W. Biederman
2022-09-20 22:21   ` Kees Cook [this message]
2022-09-20 19:24 ` Al Viro

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=202209201516.966D6EF@keescook \
    --to=keescook@chromium.org \
    --cc=ebiederm@xmission.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=renzhijie2@huawei.com \
    --cc=tanghui20@huawei.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.