Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Draft] daemon: Prevent new daemon created on same machine #646

Closed
wants to merge 1 commit into from

Conversation

minminlittleshrimp
Copy link
Collaborator

  • Only one daemon on the machine listening to fifo
  • fifo will not be unlinked by the new daemon init

src/daemon/dlt-daemon.c Fixed Show fixed Hide fixed
+ Only one daemon on the machine listening to fifo
+ fifo will not be unlinked by the new daemon init
+ Open fd and work with fchown avoiding TOCTOU

Signed-off-by: LUU QUANG MINH <[email protected]>
Signed-off-by: andv <[email protected]>
@@ -1602,13 +1606,21 @@
return -1;
} /* if */

fd = open(tmpFifo, O_RDWR);

Check failure

Code scanning / CodeQL

Time-of-check time-of-use filesystem race condition High

The
filename
being operated upon was previously
checked
, but the underlying file may have been changed since then.
@minminlittleshrimp minminlittleshrimp changed the title daemon: Prevent new daemon created on same machine [Draft] daemon: Prevent new daemon created on same machine Jun 27, 2024
@minminlittleshrimp
Copy link
Collaborator Author

Currently, the toctou issue is not yet solved, could you kindly provide further idea @duvanan13 ?

@duvanan13
Copy link
Contributor

duvanan13 commented Jul 1, 2024

Hello @minminlittleshrimp,
this is my code of dlt_daemon_init_fifo() that pass the Code Scanning/CodeQL.

static int dlt_daemon_init_fifo(DltDaemonLocal *daemon_local)
{
    int ret;
    int fd = -1;
    int fifo_size;

    /* open named pipe(FIFO) to receive DLT messages from users */
    umask(0);

    /* Valid fifo means there is a daemon running, stop init phase of the new */
    const char *tmpFifo = daemon_local->flags.daemonFifoName;
    if (access(tmpFifo, F_OK) == 0) {
        dlt_vlog(LOG_WARNING, "FIFO user %s is in use (%s)!\n",
                 tmpFifo, strerror(errno));
        return -1;
    }

    ret = mkfifo(tmpFifo, S_IRUSR | S_IWUSR | S_IWGRP);

    if (ret == -1) {
        dlt_vlog(LOG_WARNING, "FIFO user %s cannot be created (%s)!\n",
                 tmpFifo, strerror(errno));
        return -1;
    } /* if */

    const char* nameDir = "/tmp";
    int dir_fd;
    dir_fd = open(nameDir, O_RDONLY);
    if (dir_fd == -1) {
        dlt_vlog(LOG_WARNING, "Directory %s of fifo  cannot be opened (%s)!\n",
                 nameDir, strerror(errno));
        return -1;
    }

    fd = openat(dir_fd, tmpFifo, O_RDWR);

    if (fd == -1) {
        dlt_vlog(LOG_WARNING, "FIFO user %s cannot be opened (%s)!\n",
                 tmpFifo, strerror(errno));
        return -1;
    } /* if */

    /* Set group of daemon FIFO */
    if (daemon_local->flags.daemonFifoGroup[0] != 0) {
        errno = 0;
        struct group *group_dlt = getgrnam(daemon_local->flags.daemonFifoGroup);

        if (group_dlt) {
            ret = fchown(fd, -1, group_dlt->gr_gid);

            if (ret == -1)
                dlt_vlog(LOG_ERR, "FIFO user %s cannot be chowned to group %s (%s)\n",
                         tmpFifo, daemon_local->flags.daemonFifoGroup,
                         strerror(errno));
        }
        else if ((errno == 0) || (errno == ENOENT) || (errno == EBADF) || (errno == EPERM))
        {
            dlt_vlog(LOG_ERR, "Group name %s is not found (%s)\n",
                     daemon_local->flags.daemonFifoGroup,
                     strerror(errno));
        }
        else {
            dlt_vlog(LOG_ERR, "Failed to get group id of %s (%s)\n",
                     daemon_local->flags.daemonFifoGroup,
                     strerror(errno));
        }
    }
    ...

Please kindly review and send me feedback!

@minminlittleshrimp
Copy link
Collaborator Author

minminlittleshrimp commented Jul 1, 2024

Hello @duvanan13
Kindly provide the patchset view (the view with + for adding and - for removing).
Thanks

@duvanan13
Copy link
Contributor

duvanan13 commented Jul 1, 2024

Hello @minminlittleshrimp,
This is my changes: a.patch

@minminlittleshrimp
Copy link
Collaborator Author

Hello @duvanan13
Kindly create your PR and push for review.
I will close mine as duplicate.
Thanks

@minminlittleshrimp
Copy link
Collaborator Author

Close as duplicate, head to @duvanan13 PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Another dlt-dameon recreate fifo file before exit error
2 participants