Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
spawn: more CLOEXEC fixes
  • Loading branch information
@perexg
perexg committed Nov 17, 2014
1 parent cee7147 commit 2cee0bf
Show file tree
Hide file tree
Showing 7 changed files with 16 additions and 8 deletions.
2 changes: 1 addition & 1 deletion src/dvr/dvr_inotify.c
View file
Expand Up @@ -63,7 +63,7 @@ pthread_t dvr_inotify_tid;

void dvr_inotify_init ( void )
{
_inot_fd = inotify_init();
_inot_fd = inotify_init1(IN_CLOEXEC);
if (_inot_fd < 0) {
tvhlog(LOG_ERR, "dvr", "failed to initialise inotify (err=%s)",
strerror(errno));
Expand Down
2 changes: 1 addition & 1 deletion src/fsmonitor.c
View file
Expand Up @@ -105,7 +105,7 @@ void
fsmonitor_init ( void )
{
/* Intialise inotify */
fsmonitor_fd = inotify_init();
fsmonitor_fd = inotify_init1(IN_CLOEXEC);
tvhthread_create0(&fsmonitor_tid, NULL, fsmonitor_thread, NULL, "fsmonitor");
}

Expand Down
2 changes: 1 addition & 1 deletion src/input/mpegts/iptv/iptv_pipe.c
View file
Expand Up @@ -33,7 +33,7 @@
static int
iptv_pipe_start ( iptv_mux_t *im, const char *_raw, const url_t *url )
{
char *argv[32], *f, *raw, *s, *p, *a;
char *argv[64], *f, *raw, *s, *p, *a;
int i = 1, rd;

if (strncmp(_raw, "pipe://", 7))
Expand Down
2 changes: 1 addition & 1 deletion src/input/mpegts/tvhdhomerun/tvhdhomerun_frontend.c
View file
Expand Up @@ -111,7 +111,7 @@ tvhdhomerun_frontend_input_thread ( void *aux )
local_ip = hdhomerun_device_get_local_machine_addr(hfe->hf_hdhomerun_tuner);

/* first setup a local socket for the device to stream to */
sockfd = socket(AF_INET, SOCK_DGRAM, 0);
sockfd = tvh_socket(AF_INET, SOCK_DGRAM, 0);
if(sockfd == -1) {
tvherror("stvhdhomerun", "failed to open socket (%d)", errno);
return NULL;
Expand Down
12 changes: 10 additions & 2 deletions src/spawn.c
View file
Expand Up @@ -304,7 +304,7 @@ int
spawn_and_give_stdout(const char *prog, char *argv[], int *rd, int redir_stderr)
{
pid_t p;
int fd[2], f;
int fd[2], f, maxfd;
char bin[256];
const char *local_argv[2] = { NULL, NULL };

Expand All @@ -316,6 +316,8 @@ spawn_and_give_stdout(const char *prog, char *argv[], int *rd, int redir_stderr)
if (!argv) argv = (void *)local_argv;
if (!argv[0]) argv[0] = (char*)prog;

maxfd = sysconf(_SC_OPEN_MAX);

pthread_mutex_lock(&fork_lock);

if(pipe(fd) == -1) {
Expand All @@ -338,6 +340,11 @@ spawn_and_give_stdout(const char *prog, char *argv[], int *rd, int redir_stderr)
close(fd[0]);
dup2(fd[1], 1);
close(fd[1]);
if (redir_stderr)
dup2(spawn_pipe_error.wr, 2);

for (f = 3; f < maxfd; f++)
close(f);

f = open("/dev/null", O_RDWR);
if(f == -1) {
Expand All @@ -347,7 +354,8 @@ spawn_and_give_stdout(const char *prog, char *argv[], int *rd, int redir_stderr)
}

dup2(f, 0);
dup2(redir_stderr ? spawn_pipe_error.wr : f, 2);
if (!redir_stderr)
dup2(f, 2);
close(f);

spawn_info("Executing \"%s\"\n", prog);
Expand Down
2 changes: 1 addition & 1 deletion src/tvhpoll.c
View file
Expand Up @@ -70,7 +70,7 @@ tvhpoll_create ( size_t n )
{
int fd;
#if ENABLE_EPOLL
if ((fd = epoll_create(n)) < 0) {
if ((fd = epoll_create1(EPOLL_CLOEXEC)) < 0) {

This comment has been minimized.

Sign in to view

Copy link
@nurtext

nurtext Nov 28, 2014

Same here like my comment for CLOEXEC. Cross compilation fails.

src/tvhpoll.c: In function 'tvhpoll_create':
src/tvhpoll.c:73: warning: implicit declaration of function 'epoll_create1'
src/tvhpoll.c:73: error: 'EPOLL_CLOEXEC' undeclared (first use in this function)
src/tvhpoll.c:73: error: (Each undeclared identifier is reported only once
src/tvhpoll.c:73: error: for each function it appears in.)

Any chance for getting a new patch like in 36a06a6 @perexg?
tvhlog(LOG_ERR, "tvhpoll", "failed to create epoll [%s]",
strerror(errno));
return NULL;
Expand Down
2 changes: 1 addition & 1 deletion src/uuid.c
View file
Expand Up @@ -94,7 +94,7 @@ bin2hex(char *dst, size_t dstlen, const uint8_t *src, size_t srclen)
void
uuid_init ( void )
{
fd = open(RANDOM_PATH, O_RDONLY);
fd = open(RANDOM_PATH, O_RDONLY|O_CLOEXEC);

This comment has been minimized.

Sign in to view

Copy link
@nurtext

nurtext Nov 27, 2014

This will raise the following error when cross-compiling for Synology:

src/uuid.c: In function 'uuid_init':
src/uuid.c:97: error: 'O_CLOEXEC' undeclared (first use in this function)
src/uuid.c:97: error: (Each undeclared identifier is reported only once
src/uuid.c:97: error: for each function it appears in.)

Any ideas how to fix @perexg?

This comment has been minimized.

Sign in to view

Copy link
@ProfYaffle

ProfYaffle Nov 27, 2014

Member

@nurtext @perexg From what I read in response to a forum query, this is a kernel issue - it's because Synology (and some more 'appliance'-like Linux distros) are still using 2.6.x, and O_CLOEXEC was introduced into the kernel around that time.

Options I see:

  1. Don't support old kernels (not good for many platforms)
  2. Upgrade the kernel (not possible on many platforms)
  3. Don't use O_CLOEXEC in tvh (not sure of the implications)
  4. Check to see if it's defined at compile time, and then either use it or define it as a NOOP-style function (ditto implications)
  5. Define it as NOOP in the system (no implication for the system as it doesn't exist, but perhaps implications for the application as above)
if (fd == -1) {
tvherror("uuid", "failed to open %s", RANDOM_PATH);
exit(1);
Expand Down

3 comments on commit 2cee0bf

@adamsutton
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is another workaround for this, which is how the original TVH code works. Which is to use the tvh_open() wrapper to set the CLOEXEC flag using fcntl().

Setting CLOEXEC by default is good practice, since it removes the potentially for accidentally leaking access to potentially important resources.

@perexg
Copy link
Contributor Author

@perexg perexg commented on 2cee0bf Nov 27, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed the issue in uuid.c using tvh_open - 36a06a6 , but for rest, I would just add #ifdefs , Note that I figured that some linked libraries do not care (avahi, dbus, hdhomerun), so there is

for (f = 3; f < maxfd; f++)
close(f);

loop in the spawn/exec routines now..

@nurtext
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@perexg @ProfYaffle Thanks for fixing the CLOEXEC issue! I still get another CC error when trying to cross-compile for Synology. For more details see my following comment: 2cee0bf#commitcomment-8759211

If I'm experienced enough in C/C++ I would fix it by myself, but unfortunatly I'm not 😞

Please sign in to comment.