linux-kernel - Re: [PATCH 04/24] perf daemon: Add server socket support

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]

Date:   Sat, 6 Feb 2021 20:04:16 +0100
From:   Jiri Olsa <jolsa@...hat.com>
To:     Namhyung Kim <namhyung@...nel.org>
Cc:     Jiri Olsa <jolsa@...nel.org>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        lkml <linux-kernel@...r.kernel.org>,
        Peter Zijlstra <a.p.zijlstra@...llo.nl>,
        Ingo Molnar <mingo@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Michael Petlan <mpetlan@...hat.com>,
        Ian Rogers <irogers@...gle.com>,
        Stephane Eranian <eranian@...gle.com>,
        Alexei Budankov <abudankov@...wei.com>
Subject: Re: [PATCH 04/24] perf daemon: Add server socket support

On Fri, Feb 05, 2021 at 08:30:10PM +0900, Namhyung Kim wrote:
> On Sun, Jan 31, 2021 at 8:49 AM Jiri Olsa <jolsa@...nel.org> wrote:
> >
> > Add support to create server socket that listens for client
> > commands and process them.
> >
> > This patch adds only the core support, all commands using
> > this functionality are coming in following patches.
> >
> > Signed-off-by: Jiri Olsa <jolsa@...nel.org>
> > ---
> >  tools/perf/builtin-daemon.c | 101 +++++++++++++++++++++++++++++++++++-
> >  1 file changed, 100 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
> > index 8d0ac44ec808..756d60616d7d 100644
> > --- a/tools/perf/builtin-daemon.c
> > +++ b/tools/perf/builtin-daemon.c
> > @@ -1,5 +1,6 @@
> >  // SPDX-License-Identifier: GPL-2.0
> >  #include <subcmd/parse-options.h>
> > +#include <api/fd/array.h>
> >  #include <linux/limits.h>
> >  #include <string.h>
> >  #include <signal.h>
> > @@ -7,6 +8,10 @@
> >  #include <stdio.h>
> >  #include <unistd.h>
> >  #include <errno.h>
> > +#include <sys/types.h>
> > +#include <sys/socket.h>
> > +#include <sys/un.h>
> > +#include <poll.h>
> >  #include "builtin.h"
> >  #include "perf.h"
> >  #include "debug.h"
> > @@ -37,6 +42,78 @@ static void sig_handler(int sig __maybe_unused)
> >         done = true;
> >  }
> >
> > +static int setup_server_socket(struct daemon *daemon)
> > +{
> > +       struct sockaddr_un addr;
> > +       char path[100];
> > +       int fd;
> > +
> > +       fd = socket(AF_UNIX, SOCK_STREAM, 0);
> > +       if (fd < 0) {
> > +               fprintf(stderr, "socket: %s\n", strerror(errno));
> > +               return -1;
> > +       }
> > +
> > +       fcntl(fd, F_SETFD, FD_CLOEXEC);
> > +
> > +       scnprintf(path, PATH_MAX, "%s/control", daemon->base);
> 
> I couldn't find where the default value of daemon->base is set.
> Also 100 bytes seem not enough for the path name.

108 bytes is the limit of the unix socket path,
I'm adding more checks on the provided base,
so we display some reasonable error 

thanks,
jirka