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

[Jiri Olsa]

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@xxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> >  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