Hi Minh,
I think this additional check, current and parent, directory is enough
for V1 patch. The usage of the 2nd
parameter of strtol in V2 patch can be put in a utility function for a
broader use.
/Regards HansN
On 03/19/2018 08:50 AM, Minh Hon Chau wrote:
Hi Hans,
Agree that the check of "." and ".." should be added in V1.
This V2 I use the second parameter of strtol, it should ensure that
anything read from the fd directory is entirely digit, before close
the fd.
There should not be any alphabet-based directories other than ".",
".." and 0, 1, 2, ..etc, but the usage of 2nd parameter of strtol is
more generalized
So I think the check of strcmp is not needed?
Thanks,
Minh
On 19/03/18 18:00, Hans Nordebäck wrote:
Hi Minh,
my comment was that this check could be added:
if (strcmp(pentry->d_name, ".") == 0 || strcmp(pentry->d_name, "..")
== 0)
continue;
/Regards HansN
On 03/16/2018 01:27 PM, Minh Hon Chau wrote:
Hi Anders, Hans,
When I tested the patch, I did see the "." and ".." returned from
readdir, but the stroimax also return 0, so probably it won't be a
problem to close(0) more than once
But to be more safety, it should check the @second_ptr too, I will
update and send out a V2.
Thanks
Minh
On 16/03/18 23:00, Anders Widell wrote:
Hi!
See my comments below, marked AndersW2>.
regards,
Anders Widell
On 03/16/2018 12:39 PM, Hans Nordebäck wrote:
Hi Minh, ack with some comments, (on top of AndersW comments).
/Thanks HansN
On 03/15/2018 07:50 AM, Minh Chau wrote:
---
src/base/os_defs.c | 25 ++++++++++++++++++++-----
1 file changed, 20 insertions(+), 5 deletions(-)
diff --git a/src/base/os_defs.c b/src/base/os_defs.c
index 6f9ec52..e914011 100644
--- a/src/base/os_defs.c
+++ b/src/base/os_defs.c
@@ -1052,14 +1052,29 @@ uint32_t
ncs_os_process_execute_timed(NCS_OS_PROC_EXECUTE_TIMED_INFO *req)
* child */
if (getenv("OPENSAF_KEEP_FD_OPEN_AFTER_FORK") == NULL) {
/* Close all inherited file descriptors */
- int i = sysconf(_SC_OPEN_MAX);
- if (i == -1) {
+ int fd_max = sysconf(_SC_OPEN_MAX);
+
+ if (fd_max == -1) {
syslog(LOG_ERR, "%s: sysconf failed - %s",
- __FUNCTION__, strerror(errno));
+ __FUNCTION__, strerror(errno));
exit(EXIT_FAILURE);
}
- for (i--; i >= 0; --i)
- (void)close(i); /* close all descriptors */
+ struct dirent *pentry = NULL;
+ DIR *dir = opendir("/proc/self/fd");
+
+ if (dir != NULL) {
+ while ((pentry = readdir(dir)) != NULL) {
[HansN] readdir will return not only 0, 1, 2 etc. but also the
current directory '.' and '..' this should be handled here.
[HansN] perhaps we should use readdir_r instead?
AndersW2> I also thought about this, but it turns out that
readdir_r is (or will become) obsolete. It is listed as obsolete on
our wiki page:
https://sourceforge.net/p/opensaf/wiki/Unsafe%20and%20Obsolete%20Functions/
Maybe we should unlist readdir() from the non-thread-safe section?
Regarding . and .., I think we should check for parse errors, i.e.
if it was a valid number or not. The second parameter to strtoimax
will (if not NULL) tell us how much of the string that was parsed.
It should point to a '\0' character if the whole string was parsed
as a valid number. In addition, you need to check that the string
was not empty to begin with.
I am thinking about adding a support function in base, that can
parse strings into numbers and handle parse errors in a convenient
way. the strtoxxx functions are a bit tricky since you need to
check the end pointer, and also errno for overflow/underflow errors.
+ int fd = strtoimax(pentry->d_name, NULL, 10);
+ if (fd > INT_MIN && fd < fd_max)
(void)close(fd);
+ }
+ (void)closedir(dir);
+ } else {
+ /* fall back, close all possible descriptors */
+ syslog(LOG_ERR, "%s: opendir failed - %s",
+ __FUNCTION__, strerror(errno));
+ for (fd_max--; fd_max >= 0; --fd_max)
+ (void)close(fd_max);
+ }
/* Redirect standard files to /dev/null */
if (freopen("/dev/null", "r", stdin) == NULL)
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
