Message ID | 20181011204135.30058-1-karol@babioch.de |
---|---|
State | Superseded |
Headers | show |
Series | None | expand |
On Thu, 2018-10-11 at 22:41 +0200, Karol Babioch wrote: > On Linux this flag will make sure that no file descriptor is accidentally > leaked into potential child processes. While this is not a problem right now, > it is considered to be good practice these days when dealing with file > descriptors on the Linux. > > Signed-off-by: Karol Babioch <karol@babioch.de> > --- > src/utils/wpa_debug.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/src/utils/wpa_debug.c b/src/utils/wpa_debug.c > index b412f88e3..9d159632d 100644 > --- a/src/utils/wpa_debug.c > +++ b/src/utils/wpa_debug.c > @@ -60,6 +60,9 @@ static int wpa_to_android_level(int level) > #ifdef CONFIG_DEBUG_FILE > #include <sys/types.h> > #include <sys/stat.h> > +#ifdef __linux__ > +#include <fcntl.h> > +#endif /* __linux__ */ > > static int out_fd = -1; > static FILE *out_file = NULL; > @@ -567,6 +570,14 @@ int wpa_debug_open_file(const char *path) > return -1; > } > > +#ifdef __linux__ > + if (fcntl(out_fd, F_SETFD, FD_CLOEXEC) == -1) { > + wpa_printf(MSG_ERROR, "wpa_debug_open_file: Failed to set O_CLOEXEC " > + "on output file descriptor, using standard output"); I'd argue you should just fall back to the old behaviour in the unlikely even this fails, rather than closing the FD? johannes
Hi Johannes, Am 12.10.18 um 07:52 schrieb Johannes Berg: > I'd argue you should just fall back to the old behaviour in the unlikely > even this fails, rather than closing the FD? Of course your suggestion makes a lot of sense and I will send an updatech patch. Do you have any opinion on what kind of level the message that we output in this case should be? MSG_ERROR might be overkill, since we continue to work. Not sure if this is MSG_WARNING or whether we should go below that. As you've said it is extremely unlikely that this is going to fail. Best regards, Karol Babioch
diff --git a/src/utils/wpa_debug.c b/src/utils/wpa_debug.c index b412f88e3..9d159632d 100644 --- a/src/utils/wpa_debug.c +++ b/src/utils/wpa_debug.c @@ -60,6 +60,9 @@ static int wpa_to_android_level(int level) #ifdef CONFIG_DEBUG_FILE #include <sys/types.h> #include <sys/stat.h> +#ifdef __linux__ +#include <fcntl.h> +#endif /* __linux__ */ static int out_fd = -1; static FILE *out_file = NULL; @@ -567,6 +570,14 @@ int wpa_debug_open_file(const char *path) return -1; } +#ifdef __linux__ + if (fcntl(out_fd, F_SETFD, FD_CLOEXEC) == -1) { + wpa_printf(MSG_ERROR, "wpa_debug_open_file: Failed to set O_CLOEXEC " + "on output file descriptor, using standard output"); + close(out_fd); + return -1; + } +#endif /* __linux__ */ #ifndef _WIN32 setvbuf(out_file, NULL, _IOLBF, 0); #endif /* _WIN32 */
On Linux this flag will make sure that no file descriptor is accidentally leaked into potential child processes. While this is not a problem right now, it is considered to be good practice these days when dealing with file descriptors on the Linux. Signed-off-by: Karol Babioch <karol@babioch.de> --- src/utils/wpa_debug.c | 11 +++++++++++ 1 file changed, 11 insertions(+)