linux-kernel - Re: [PATCH] Added PR_SET_PROCTITLE

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: <3e8340490910021901i35ba8a5v7647b1e02b054270@mail.gmail.com>
Date:	Fri, 2 Oct 2009 22:01:17 -0400
From:	Bryan Donlan <bdonlan@...il.com>
To:	Timo Sirainen <tss@....fi>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Added PR_SET_PROCTITLE_AREA option for prctl()

On Fri, Oct 2, 2009 at 5:23 PM, Timo Sirainen <tss@....fi> wrote:
> PR_SET_PROCTITLE_AREA updates mm_struct->arg_start and arg_end to the
> given pointers, which makes it possible for user space to implement
> setproctitle(3) cleanly.


> @@ -267,9 +267,12 @@ static int proc_pid_cmdline(struct task_struct *task, char * buffer)
>
>        res = access_process_vm(task, mm->arg_start, buffer, len, 0);
>
> -       // If the nul at the end of args has been overwritten, then
> -       // assume application is using setproctitle(3).
> -       if (res > 0 && buffer[res-1] != '\0' && len < PAGE_SIZE) {
> +       if (mm->arg_end != mm->env_start) {
> +               // PR_SET_PROCTITLE_AREA used
> +               res = strnlen(buffer, res);

Is this check really needed? Surely it's enough to simply state that
behavior if the area isn't null-terminated is undefined.

> +       } else if (res > 0 && buffer[res-1] != '\0' && len < PAGE_SIZE) {
> +               // If the nul at the end of args has been overwritten, then
> +               // assume application is using old style setproctitle(3).
>                len = strnlen(buffer, res);
>                if (len < res) {
>                    res = len;

Might want to fix the bug later on in that function while you're in
here - the second access_process_vm call is never checked for errors,
but (from my reading) it's possible that the page that the environment
is on could be unmapped between those two calls. The result could
either be a short read (not the end of the world) or a negative value
(error code + small original argument length) passed to strnlen.

That said, come to think of it, I'm not actually sure if this prctl
stuff is strictly necessary. Wouldn't it be enough for glibc to copy
the environment somewhere safe, and then have the kernel guarantee a
full PAGE_SIZE between arg_start and env_end, even if this means
padding out the environment? The process could then measure to make
sure it has this much space (in case of running on an old kernel) by
testing the difference between arg_start and the top of the stack, or
an auxiliary vector could be passed down from the kernel with the
maximum proctitle length.
--
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/