On Thu, 2010-09-30 at 23:37 +0200, Karel Zak wrote:
> On Mon, Sep 27, 2010 at 07:08:47PM -0400, Davidlohr Bueso wrote:
> > taskset: provide proper pid validation.
> >
> > When passing the -p option, atoi() does not do a good enough job passing string based number to a proper intenger.
>
> Yep, I have applied a little different version of the patch. Thanks.
>
> Karel
>
>
>
> >From 24295096586c848a5eb3f8d8b73a73f03c174fc1 Mon Sep 17 00:00:00 2001
> From: Karel Zak <kzak@xxxxxxxxxx>
> Date: Thu, 30 Sep 2010 23:27:42 +0200
> Subject: [PATCH] taskset: proper numbers parsing
>
> Reported-by: Davidlohr Bueso <dave@xxxxxxx>
> Signed-off-by: Karel Zak <kzak@xxxxxxxxxx>
> ---
> schedutils/Makefile.am | 7 ++++++-
> schedutils/chrt.c | 10 ++++------
> schedutils/ionice.c | 34 +++++++---------------------------
> schedutils/schedutils.c | 34 ++++++++++++++++++++++++++++++++++
> schedutils/schedutils.h | 7 +++++++
> schedutils/taskset.c | 4 +++-
I had thought of that too since getnum() is used by several programs. We
can actually now think of replacing atoi() in utils-linux-ng with this.
Thanks,
Davidlohr
> 6 files changed, 61 insertions(+), 35 deletions(-)
> create mode 100644 schedutils/schedutils.c
> create mode 100644 schedutils/schedutils.h
>
> diff --git a/schedutils/Makefile.am b/schedutils/Makefile.am
> index 13d45c8..c83e5ea 100644
> --- a/schedutils/Makefile.am
> +++ b/schedutils/Makefile.am
> @@ -2,19 +2,24 @@ include $(top_srcdir)/config/include-Makefile.am
>
> if BUILD_SCHEDUTILS
>
> +srcs_common = schedutils.c schedutils.h
> +
> usrbin_exec_PROGRAMS = chrt
> dist_man_MANS = chrt.1
>
> +chrt_SOURCES = chrt.c $(srcs_common)
> +
> if HAVE_IOPRIO_GET
> if HAVE_IOPRIO_SET
> usrbin_exec_PROGRAMS += ionice
> +ionice_SOURCES = ionice.c $(srcs_common)
> dist_man_MANS += ionice.1
> endif
> endif
>
> if HAVE_SCHED_GETAFFINITY
> usrbin_exec_PROGRAMS += taskset
> -taskset_SOURCES = taskset.c $(top_srcdir)/lib/cpuset.c
> +taskset_SOURCES = taskset.c $(top_srcdir)/lib/cpuset.c $(srcs_common)
> dist_man_MANS += taskset.1
> endif
>
> diff --git a/schedutils/chrt.c b/schedutils/chrt.c
> index 2d0254f..89b12c7 100644
> --- a/schedutils/chrt.c
> +++ b/schedutils/chrt.c
> @@ -32,6 +32,8 @@
> #include "c.h"
> #include "nls.h"
>
> +#include "schedutils.h"
> +
> /* the SCHED_BATCH is supported since Linux 2.6.16
> * -- temporary workaround for people with old glibc headers
> */
> @@ -238,9 +240,7 @@ int main(int argc, char *argv[])
> break;
> case 'p':
> errno = 0;
> - pid = strtol(argv[argc - 1], NULL, 10);
> - if (errno)
> - err(EXIT_FAILURE, _("failed to parse pid"));
> + pid = getnum(argv[argc - 1], _("failed to parse pid"));
> break;
> case 'r':
> policy = SCHED_RR;
> @@ -268,9 +268,7 @@ int main(int argc, char *argv[])
> }
>
> errno = 0;
> - priority = strtol(argv[optind], NULL, 10);
> - if (errno)
> - err(EXIT_FAILURE, _("failed to parse priority"));
> + priority = getnum(argv[optind], _("failed to parse priority"));
>
> #ifdef SCHED_RESET_ON_FORK
> /* sanity check */
> diff --git a/schedutils/ionice.c b/schedutils/ionice.c
> index 3ad4ef9..34132f0 100644
> --- a/schedutils/ionice.c
> +++ b/schedutils/ionice.c
> @@ -18,6 +18,8 @@
>
> #include "nls.h"
>
> +#include "schedutils.h"
> +
> static int tolerant;
>
> static inline int ioprio_set(int which, int who, int ioprio)
> @@ -91,28 +93,6 @@ static void usage(int rc)
> exit(rc);
> }
>
> -static long getnum(const char *str)
> -{
> - long num;
> - char *end = NULL;
> -
> - if (str == NULL || *str == '\0')
> - goto err;
> - errno = 0;
> - num = strtol(str, &end, 10);
> -
> - if (errno || (end && *end))
> - goto err;
> -
> - return num;
> -err:
> - if (errno)
> - err(EXIT_SUCCESS, _("cannot parse number '%s'"), str);
> - else
> - errx(EXIT_SUCCESS, _("cannot parse number '%s'"), str);
> - return 0;
> -}
> -
> int main(int argc, char *argv[])
> {
> int ioprio = 4, set = 0, ioclass = IOPRIO_CLASS_BE, c;
> @@ -125,15 +105,15 @@ int main(int argc, char *argv[])
> while ((c = getopt(argc, argv, "+n:c:p:th")) != EOF) {
> switch (c) {
> case 'n':
> - ioprio = getnum(optarg);
> + ioprio = getnum(optarg, _("failed to parse class data"));
> set |= 1;
> break;
> case 'c':
> - ioclass = getnum(optarg);
> + ioclass = getnum(optarg, _("failed to parse class"));
> set |= 2;
> break;
> case 'p':
> - pid = getnum(optarg);
> + pid = getnum(optarg, _("failed to parse pid"));
> break;
> case 't':
> tolerant = 1;
> @@ -167,7 +147,7 @@ int main(int argc, char *argv[])
> ioprio_print(pid);
>
> for(; argv[optind]; ++optind) {
> - pid = getnum(argv[optind]);
> + pid = getnum(argv[optind], _("failed to parse pid"));
> ioprio_print(pid);
> }
> } else {
> @@ -176,7 +156,7 @@ int main(int argc, char *argv[])
>
> for(; argv[optind]; ++optind)
> {
> - pid = getnum(argv[optind]);
> + pid = getnum(argv[optind], _("failed to parse pid"));
> ioprio_setpid(pid, ioprio, ioclass);
> }
> }
> diff --git a/schedutils/schedutils.c b/schedutils/schedutils.c
> new file mode 100644
> index 0000000..9d6051b
> --- /dev/null
> +++ b/schedutils/schedutils.c
> @@ -0,0 +1,34 @@
> +/*
> + * Copyright (C) 2010 Karel Zak <kzak@xxxxxxxxxx>
> + *
> + * Released under the terms of the GNU General Public License version 2
> + *
> + */
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <errno.h>
> +#include <err.h>
> +
> +#include "nls.h"
> +
> +long getnum(const char *str, const char *errmesg)
> +{
> + long num;
> + char *end = NULL;
> +
> + if (str == NULL || *str == '\0')
> + goto err;
> + errno = 0;
> + num = strtol(str, &end, 10);
> +
> + if (errno || (end && *end))
> + goto err;
> +
> + return num;
> +err:
> + if (errno)
> + err(EXIT_FAILURE, "%s: '%s'", errmesg, str);
> + else
> + errx(EXIT_FAILURE, "%s: '%s'", errmesg, str);
> + return 0;
> +}
> diff --git a/schedutils/schedutils.h b/schedutils/schedutils.h
> new file mode 100644
> index 0000000..80e4f7b
> --- /dev/null
> +++ b/schedutils/schedutils.h
> @@ -0,0 +1,7 @@
> +#ifndef UTIL_LINUX_SCHED_UTILS
> +#define UTIL_LINUX_SCHED_UTILS
> +
> +extern long getnum(const char *str, const char *errmesg);
> +
> +#endif
> +
> diff --git a/schedutils/taskset.c b/schedutils/taskset.c
> index 2f1bc74..201fc15 100644
> --- a/schedutils/taskset.c
> +++ b/schedutils/taskset.c
> @@ -32,6 +32,8 @@
> #include "cpuset.h"
> #include "nls.h"
>
> +#include "schedutils.h"
> +
> static void __attribute__((__noreturn__)) usage(FILE *out)
> {
> fprintf(out,
> @@ -87,7 +89,7 @@ int main(int argc, char *argv[])
> while ((opt = getopt_long(argc, argv, "+pchV", longopts, NULL)) != -1) {
> switch (opt) {
> case 'p':
> - pid = atoi(argv[argc - 1]);
> + pid = getnum(argv[argc - 1], _("failed to parse pid"));
> break;
> case 'c':
> c_opt = 1;
--
To unsubscribe from this list: send the line "unsubscribe util-linux-ng" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
