Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 19 Apr 2015 21:59:46 +0200
From:      Oliver Pinter <oliver.pinter@hardenedbsd.org>
To:        Konstantin Belousov <kostikbel@gmail.com>
Cc:        "freebsd-arch@freebsd.org" <arch@freebsd.org>, peter@freebsd.org
Subject:   Re: setproctitle [was: Re: Removal of the 6.x kernel compat code from libc]
Message-ID:  <CAPQ4ffvn49iXeazCRYhYCrorkTQe4Tymwz6Sd41VqWTrYgRiZg@mail.gmail.com>
In-Reply-To: <20150419085248.GB2390@kib.kiev.ua>
References:  <20150417075942.GI2390@kib.kiev.ua> <CAPQ4ffsrMNLBChrUc5wBrY%2BnSwnfZhBqSp%2BhES0tpLUJi-bXow@mail.gmail.com> <20150419085248.GB2390@kib.kiev.ua>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, Apr 19, 2015 at 10:52 AM, Konstantin Belousov
<kostikbel@gmail.com> wrote:
> On Fri, Apr 17, 2015 at 01:39:04PM +0200, Oliver Pinter wrote:
>> Is there any chanche to get ride of the very old (FreeBSD 2.x) compat
>> hacks like these:
>> https://github.com/freebsd/freebsd/blob/master/lib/libc/gen/setproctitle.c#L40 ?
>>
>
> Below is the promised cleanup of the libc/gen/setproctitle.c.

Seems fine to me, and I started a jenkins build with this patch:
http://jenkins.hardenedbsd.org:8180/jenkins/job/HardenedBSD-unstable-amd64/changes

>
> I see a value in keeping the track of the historical interfaces. I
> considered moving the explanation about old_ps_strings to sys/exec.h,
> where the current ps_strings is defined and explained, but this would
> make the safety checks in the setproctitle() code uncomprehensive.

Fine.

>
> diff --git a/lib/libc/gen/setproctitle.c b/lib/libc/gen/setproctitle.c
> index cd705fb..9dff328 100644
> --- a/lib/libc/gen/setproctitle.c
> +++ b/lib/libc/gen/setproctitle.c
> @@ -42,9 +42,10 @@ __FBSDID("$FreeBSD$");
>   * 1: old_ps_strings at the very top of the stack.
>   * 2: old_ps_strings at SPARE_USRSPACE below the top of the stack.
>   * 3: ps_strings at the very top of the stack.
> - * This attempts to support a kernel built in the #2 and #3 era.
> - */
> -
> + * We only support a kernel providing #3 style ps_strings.
> + *
> + * For historical purposes, a definition of the old ps_strings structure
> + * and location is preserved below:
>  struct old_ps_strings {
>         char    *old_ps_argvstr;
>         int     old_ps_nargvstr;
> @@ -53,6 +54,7 @@ struct old_ps_strings {
>  };
>  #define        OLD_PS_STRINGS ((struct old_ps_strings *) \
>         (USRSTACK - SPARE_USRSPACE - sizeof(struct old_ps_strings)))
> + */
>
>  #include <stdarg.h>
>
> @@ -136,41 +138,38 @@ setproctitle(const char *fmt, ...)
>                 ps_strings = (struct ps_strings *)ul_ps_strings;
>         }
>
> -       /* PS_STRINGS points to zeroed memory on a style #2 kernel */
> -       if (ps_strings->ps_argvstr) {
> -               /* style #3 */
> -               if (oargc == -1) {
> -                       /* Record our original args */
> -                       oargc = ps_strings->ps_nargvstr;
> -                       oargv = ps_strings->ps_argvstr;
> -                       for (i = len = 0; i < oargc; i++) {
> -                               /*
> -                                * The program may have scribbled into its
> -                                * argv array, e.g., to remove some arguments.
> -                                * If that has happened, break out before
> -                                * trying to call strlen on a NULL pointer.
> -                                */
> -                               if (oargv[i] == NULL) {
> -                                       oargc = i;
> -                                       break;
> -                               }
> -                               snprintf(obuf + len, SPT_BUFSIZE - len, "%s%s",
> -                                   len ? " " : "", oargv[i]);
> -                               if (len)
> -                                       len++;
> -                               len += strlen(oargv[i]);
> -                               if (len >= SPT_BUFSIZE)
> -                                       break;
> +       /*
> +        * PS_STRINGS points to zeroed memory on a style #2 kernel.
> +        * Should not happen.
> +        */
> +       if (ps_strings->ps_argvstr == NULL)
> +               return;
> +
> +       /* style #3 */
> +       if (oargc == -1) {
> +               /* Record our original args */
> +               oargc = ps_strings->ps_nargvstr;
> +               oargv = ps_strings->ps_argvstr;
> +               for (i = len = 0; i < oargc; i++) {
> +                       /*
> +                        * The program may have scribbled into its
> +                        * argv array, e.g., to remove some arguments.
> +                        * If that has happened, break out before
> +                        * trying to call strlen on a NULL pointer.
> +                        */
> +                       if (oargv[i] == NULL) {
> +                               oargc = i;
> +                               break;
>                         }
> +                       snprintf(obuf + len, SPT_BUFSIZE - len, "%s%s",
> +                           len != 0 ? " " : "", oargv[i]);
> +                       if (len != 0)
> +                               len++;
> +                       len += strlen(oargv[i]);
> +                       if (len >= SPT_BUFSIZE)
> +                               break;
>                 }
> -               ps_strings->ps_nargvstr = nargc;
> -               ps_strings->ps_argvstr = nargvp;
> -       } else {
> -               /* style #2 - we can only restore our first arg :-( */
> -               if (*obuf == '\0')
> -                       strncpy(obuf, OLD_PS_STRINGS->old_ps_argvstr,
> -                           SPT_BUFSIZE - 1);
> -               OLD_PS_STRINGS->old_ps_nargvstr = 1;
> -               OLD_PS_STRINGS->old_ps_argvstr = nargvp[0];
>         }
> +       ps_strings->ps_nargvstr = nargc;
> +       ps_strings->ps_argvstr = nargvp;
>  }



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAPQ4ffvn49iXeazCRYhYCrorkTQe4Tymwz6Sd41VqWTrYgRiZg>