[meta-oe] utils.c: close all file descriptors after completing a ptest

Submitted by Sakib Sajal on May 31, 2019, 5:44 p.m. | Patch ID: 161845

Details

Message ID 20190531174440.18333-1-sakib.sajal@windriver.com
State Superseded
Headers show

Commit Message

Sakib Sajal May 31, 2019, 5:44 p.m.
vredir ptest fails since too many file descriptors were open.

From the failed ptest log:
   run-vredir
   87,88c87,88
   < ./vredir6.sub: line 10: /dev/null: Too many open files
   < ./vredir6.sub: line 13: /dev/null: Too many open files
   FAIL: run-vredir

Added function to close file descriptors before starting a new process.

Signed-off-by: Sakib Sajal <sakib.sajal@windriver.com>
Signed-off-by: Randy Macleod <randy.macleod@windriver.com>
---
 utils.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Patch hide | download patch | download mbox

diff --git a/utils.c b/utils.c
index 504df0b..05c2bfe 100644
--- a/utils.c
+++ b/utils.c
@@ -28,6 +28,7 @@ 
 #include <fcntl.h>
 #include <time.h>
 #include <dirent.h>
+#include <sys/resource.h>
 #include <sys/types.h>
 #include <sys/wait.h>
 #include <sys/stat.h>
@@ -240,6 +241,23 @@  filter_ptests(struct ptest_list *head, char **ptests, int ptest_num)
 	return head_new;
 }
 
+/* Close all fds from 3 up to 'ulimit -n'
+ * i.e. do not close STDIN, STDOUT, STDERR.
+ * Typically called in in a child process after forking
+ * but before exec as a good policy especially for security.
+ */ 
+static void
+close_fds(void)
+{
+	struct rlimit curr_lim;
+	getrlimit(RLIMIT_NOFILE, &curr_lim);
+
+	int fd;
+	for (fd=3; fd < curr_lim.rlim_cur; fd++) {
+		(void) close(fd);
+   	}
+}
+
 static inline void
 run_child(char *run_ptest, int fd_stdout, int fd_stderr)
 {
@@ -252,6 +270,7 @@  run_child(char *run_ptest, int fd_stdout, int fd_stderr)
 	dup2(fd_stdout, STDOUT_FILENO);
 	// XXX: Redirect stderr to stdout to avoid buffer ordering problems.
 	dup2(fd_stdout, STDERR_FILENO);
+	close_fds();
 	execv(run_ptest, argv);
 
 	exit(1);

Comments

Randy MacLeod May 31, 2019, 5:53 p.m.
Thanks Sakib.

Next time please use --prefix to add the ptest-runner tag:
so you'll have a subject line like:

[meta-oe][ptest-runner] utils.c: close all file descriptors after 
completing a ptest

The subject line is wrongg since we are closing the fds
*before* the tests. Resend with something like:
    utils.c: close all fds in child for each ptest


Wait to see if Anibal has any comment then send a v2
with a 'v2' prefix in the subject like:
    [yocto] [ptest-runner][PATCHv2 1/3] Makefile: libcheck now requires 
to link subunit

Thanks,
../Randy

On 5/31/19 1:44 PM, Sakib Sajal wrote:
> vredir ptest fails since too many file descriptors were open.
> 
>  From the failed ptest log:
>     run-vredir
>     87,88c87,88
>     < ./vredir6.sub: line 10: /dev/null: Too many open files
>     < ./vredir6.sub: line 13: /dev/null: Too many open files
>     FAIL: run-vredir
> 
> Added function to close file descriptors before starting a new process.
> 
> Signed-off-by: Sakib Sajal <sakib.sajal@windriver.com>
> Signed-off-by: Randy Macleod <randy.macleod@windriver.com>
> ---
>   utils.c | 19 +++++++++++++++++++
>   1 file changed, 19 insertions(+)
> 
> diff --git a/utils.c b/utils.c
> index 504df0b..05c2bfe 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -28,6 +28,7 @@
>   #include <fcntl.h>
>   #include <time.h>
>   #include <dirent.h>
> +#include <sys/resource.h>
>   #include <sys/types.h>
>   #include <sys/wait.h>
>   #include <sys/stat.h>
> @@ -240,6 +241,23 @@ filter_ptests(struct ptest_list *head, char **ptests, int ptest_num)
>   	return head_new;
>   }
>   
> +/* Close all fds from 3 up to 'ulimit -n'
> + * i.e. do not close STDIN, STDOUT, STDERR.
> + * Typically called in in a child process after forking
> + * but before exec as a good policy especially for security.
> + */
> +static void
> +close_fds(void)
> +{
> +	struct rlimit curr_lim;
> +	getrlimit(RLIMIT_NOFILE, &curr_lim);
> +
> +	int fd;
> +	for (fd=3; fd < curr_lim.rlim_cur; fd++) {
> +		(void) close(fd);
> +   	}
> +}
> +
>   static inline void
>   run_child(char *run_ptest, int fd_stdout, int fd_stderr)
>   {
> @@ -252,6 +270,7 @@ run_child(char *run_ptest, int fd_stdout, int fd_stderr)
>   	dup2(fd_stdout, STDOUT_FILENO);
>   	// XXX: Redirect stderr to stdout to avoid buffer ordering problems.
>   	dup2(fd_stdout, STDERR_FILENO);
> +	close_fds();
>   	execv(run_ptest, argv);
>   
>   	exit(1);
>
Aníbal Limón May 31, 2019, 9:15 p.m.
LGTM, pushed.

http://git.yoctoproject.org/cgit/cgit.cgi/ptest-runner2/commit/?id=fb93c99e1e771a8ada0476b76da6f2ba665e03ba

Thanks!,
Anibal

On Fri, 31 May 2019 at 12:53, Randy MacLeod <randy.macleod@windriver.com>
wrote:

> Thanks Sakib.
>
> Next time please use --prefix to add the ptest-runner tag:
> so you'll have a subject line like:
>
> [meta-oe][ptest-runner] utils.c: close all file descriptors after
> completing a ptest
>
> The subject line is wrongg since we are closing the fds
> *before* the tests. Resend with something like:
>     utils.c: close all fds in child for each ptest
>
>
> Wait to see if Anibal has any comment then send a v2
> with a 'v2' prefix in the subject like:
>     [yocto] [ptest-runner][PATCHv2 1/3] Makefile: libcheck now requires
> to link subunit
>
> Thanks,
> ../Randy
>
> On 5/31/19 1:44 PM, Sakib Sajal wrote:
> > vredir ptest fails since too many file descriptors were open.
> >
> >  From the failed ptest log:
> >     run-vredir
> >     87,88c87,88
> >     < ./vredir6.sub: line 10: /dev/null: Too many open files
> >     < ./vredir6.sub: line 13: /dev/null: Too many open files
> >     FAIL: run-vredir
> >
> > Added function to close file descriptors before starting a new process.
> >
> > Signed-off-by: Sakib Sajal <sakib.sajal@windriver.com>
> > Signed-off-by: Randy Macleod <randy.macleod@windriver.com>
> > ---
> >   utils.c | 19 +++++++++++++++++++
> >   1 file changed, 19 insertions(+)
> >
> > diff --git a/utils.c b/utils.c
> > index 504df0b..05c2bfe 100644
> > --- a/utils.c
> > +++ b/utils.c
> > @@ -28,6 +28,7 @@
> >   #include <fcntl.h>
> >   #include <time.h>
> >   #include <dirent.h>
> > +#include <sys/resource.h>
> >   #include <sys/types.h>
> >   #include <sys/wait.h>
> >   #include <sys/stat.h>
> > @@ -240,6 +241,23 @@ filter_ptests(struct ptest_list *head, char
> **ptests, int ptest_num)
> >       return head_new;
> >   }
> >
> > +/* Close all fds from 3 up to 'ulimit -n'
> > + * i.e. do not close STDIN, STDOUT, STDERR.
> > + * Typically called in in a child process after forking
> > + * but before exec as a good policy especially for security.
> > + */
> > +static void
> > +close_fds(void)
> > +{
> > +     struct rlimit curr_lim;
> > +     getrlimit(RLIMIT_NOFILE, &curr_lim);
> > +
> > +     int fd;
> > +     for (fd=3; fd < curr_lim.rlim_cur; fd++) {
> > +             (void) close(fd);
> > +     }
> > +}
> > +
> >   static inline void
> >   run_child(char *run_ptest, int fd_stdout, int fd_stderr)
> >   {
> > @@ -252,6 +270,7 @@ run_child(char *run_ptest, int fd_stdout, int
> fd_stderr)
> >       dup2(fd_stdout, STDOUT_FILENO);
> >       // XXX: Redirect stderr to stdout to avoid buffer ordering
> problems.
> >       dup2(fd_stdout, STDERR_FILENO);
> > +     close_fds();
> >       execv(run_ptest, argv);
> >
> >       exit(1);
> >
>
>
> --
> # Randy MacLeod
> # Wind River Linux
>
Sakib Sajal June 7, 2019, 1:27 a.m.
This is a system generated Comment: Patch 161845 was automatically marked as superseded by patch 162018.