* [PATCH] Close etab file's file descriptor on stat error. @ 2015-10-28 0:01 Malahal Naineni 2015-10-28 13:53 ` J. Bruce Fields 2015-11-04 21:51 ` Steve Dickson 0 siblings, 2 replies; 4+ messages in thread From: Malahal Naineni @ 2015-10-28 0:01 UTC (permalink / raw) To: linux-nfs; +Cc: Malahal Naineni Also, fixed erroneously closing file descriptor 0 at init time. Signed-off-by: Malahal Naineni <malahal@us.ibm.com> --- utils/mountd/auth.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/utils/mountd/auth.c b/utils/mountd/auth.c index 330cab5..894a7a5 100644 --- a/utils/mountd/auth.c +++ b/utils/mountd/auth.c @@ -85,7 +85,7 @@ auth_reload() { struct stat stb; static ino_t last_inode; - static int last_fd; + static int last_fd = -1; static unsigned int counter; int fd; @@ -93,11 +93,22 @@ auth_reload() xlog(L_FATAL, "couldn't open %s", _PATH_ETAB); } else if (fstat(fd, &stb) < 0) { xlog(L_FATAL, "couldn't stat %s", _PATH_ETAB); - } else if (stb.st_ino == last_inode) { + close(fd); + } else if (last_fd != -1 && stb.st_ino == last_inode) { + /* We opened the etab file before, and its inode + * number hasn't changed since then. + */ close(fd); return counter; } else { - close(last_fd); + /* Need to process entries from the etab file. Close + * the file descriptor from the previous open (last_fd), + * and keep the current file descriptor open to prevent + * the file system reusing the current inode number + * (last_inode). + */ + if (last_fd != -1) + close(last_fd); last_fd = fd; last_inode = stb.st_ino; } -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] Close etab file's file descriptor on stat error. 2015-10-28 0:01 [PATCH] Close etab file's file descriptor on stat error Malahal Naineni @ 2015-10-28 13:53 ` J. Bruce Fields 2015-10-28 16:16 ` Malahal Naineni 2015-11-04 21:51 ` Steve Dickson 1 sibling, 1 reply; 4+ messages in thread From: J. Bruce Fields @ 2015-10-28 13:53 UTC (permalink / raw) To: Malahal Naineni; +Cc: linux-nfs, steved On Tue, Oct 27, 2015 at 07:01:30PM -0500, Malahal Naineni wrote: > Also, fixed erroneously closing file descriptor 0 at init time. Thanks, that's interesting. Did that extra close(0) have any user-visible consequences? > > Signed-off-by: Malahal Naineni <malahal@us.ibm.com> > --- > utils/mountd/auth.c | 17 ++++++++++++++--- > 1 file changed, 14 insertions(+), 3 deletions(-) > > diff --git a/utils/mountd/auth.c b/utils/mountd/auth.c > index 330cab5..894a7a5 100644 > --- a/utils/mountd/auth.c > +++ b/utils/mountd/auth.c > @@ -85,7 +85,7 @@ auth_reload() > { > struct stat stb; > static ino_t last_inode; > - static int last_fd; > + static int last_fd = -1; > static unsigned int counter; > int fd; > > @@ -93,11 +93,22 @@ auth_reload() > xlog(L_FATAL, "couldn't open %s", _PATH_ETAB); > } else if (fstat(fd, &stb) < 0) { > xlog(L_FATAL, "couldn't stat %s", _PATH_ETAB); > - } else if (stb.st_ino == last_inode) { > + close(fd); > + } else if (last_fd != -1 && stb.st_ino == last_inode) { I think that (last_fd != -1) is actually unnecessary (basically because last_inode is initially 0, which isn't a legal inode number), but... it's probably clearer this way, OK. Patch looks OK to me.--b. > + /* We opened the etab file before, and its inode > + * number hasn't changed since then. > + */ > close(fd); > return counter; > } else { > - close(last_fd); > + /* Need to process entries from the etab file. Close > + * the file descriptor from the previous open (last_fd), > + * and keep the current file descriptor open to prevent > + * the file system reusing the current inode number > + * (last_inode). > + */ > + if (last_fd != -1) > + close(last_fd); > last_fd = fd; > last_inode = stb.st_ino; > } > -- > 1.8.3.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Close etab file's file descriptor on stat error. 2015-10-28 13:53 ` J. Bruce Fields @ 2015-10-28 16:16 ` Malahal Naineni 0 siblings, 0 replies; 4+ messages in thread From: Malahal Naineni @ 2015-10-28 16:16 UTC (permalink / raw) To: J. Bruce Fields; +Cc: linux-nfs, steved I wondered that myself! gdb (of course, running in foreground) showed 0,1,2 as a pty as expected. file descriptor 3 was some socket! Maybe, I will debug it to find out what socket that was and what issues a user could get into! Regards, Malahal. J. Bruce Fields [bfields@fieldses.org] wrote: > On Tue, Oct 27, 2015 at 07:01:30PM -0500, Malahal Naineni wrote: > > Also, fixed erroneously closing file descriptor 0 at init time. > > Thanks, that's interesting. Did that extra close(0) have any > user-visible consequences? > > > > > Signed-off-by: Malahal Naineni <malahal@us.ibm.com> > > --- > > utils/mountd/auth.c | 17 ++++++++++++++--- > > 1 file changed, 14 insertions(+), 3 deletions(-) > > > > diff --git a/utils/mountd/auth.c b/utils/mountd/auth.c > > index 330cab5..894a7a5 100644 > > --- a/utils/mountd/auth.c > > +++ b/utils/mountd/auth.c > > @@ -85,7 +85,7 @@ auth_reload() > > { > > struct stat stb; > > static ino_t last_inode; > > - static int last_fd; > > + static int last_fd = -1; > > static unsigned int counter; > > int fd; > > > > @@ -93,11 +93,22 @@ auth_reload() > > xlog(L_FATAL, "couldn't open %s", _PATH_ETAB); > > } else if (fstat(fd, &stb) < 0) { > > xlog(L_FATAL, "couldn't stat %s", _PATH_ETAB); > > - } else if (stb.st_ino == last_inode) { > > + close(fd); > > + } else if (last_fd != -1 && stb.st_ino == last_inode) { > > I think that (last_fd != -1) is actually unnecessary (basically because > last_inode is initially 0, which isn't a legal inode number), but... > it's probably clearer this way, OK. > > Patch looks OK to me.--b. > > > + /* We opened the etab file before, and its inode > > + * number hasn't changed since then. > > + */ > > close(fd); > > return counter; > > } else { > > - close(last_fd); > > + /* Need to process entries from the etab file. Close > > + * the file descriptor from the previous open (last_fd), > > + * and keep the current file descriptor open to prevent > > + * the file system reusing the current inode number > > + * (last_inode). > > + */ > > + if (last_fd != -1) > > + close(last_fd); > > last_fd = fd; > > last_inode = stb.st_ino; > > } > > -- > > 1.8.3.1 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Close etab file's file descriptor on stat error. 2015-10-28 0:01 [PATCH] Close etab file's file descriptor on stat error Malahal Naineni 2015-10-28 13:53 ` J. Bruce Fields @ 2015-11-04 21:51 ` Steve Dickson 1 sibling, 0 replies; 4+ messages in thread From: Steve Dickson @ 2015-11-04 21:51 UTC (permalink / raw) To: Malahal Naineni, linux-nfs On 10/27/2015 08:01 PM, Malahal Naineni wrote: > Also, fixed erroneously closing file descriptor 0 at init time. > > Signed-off-by: Malahal Naineni <malahal@us.ibm.com> Committed... steved. > --- > utils/mountd/auth.c | 17 ++++++++++++++--- > 1 file changed, 14 insertions(+), 3 deletions(-) > > diff --git a/utils/mountd/auth.c b/utils/mountd/auth.c > index 330cab5..894a7a5 100644 > --- a/utils/mountd/auth.c > +++ b/utils/mountd/auth.c > @@ -85,7 +85,7 @@ auth_reload() > { > struct stat stb; > static ino_t last_inode; > - static int last_fd; > + static int last_fd = -1; > static unsigned int counter; > int fd; > > @@ -93,11 +93,22 @@ auth_reload() > xlog(L_FATAL, "couldn't open %s", _PATH_ETAB); > } else if (fstat(fd, &stb) < 0) { > xlog(L_FATAL, "couldn't stat %s", _PATH_ETAB); > - } else if (stb.st_ino == last_inode) { > + close(fd); > + } else if (last_fd != -1 && stb.st_ino == last_inode) { > + /* We opened the etab file before, and its inode > + * number hasn't changed since then. > + */ > close(fd); > return counter; > } else { > - close(last_fd); > + /* Need to process entries from the etab file. Close > + * the file descriptor from the previous open (last_fd), > + * and keep the current file descriptor open to prevent > + * the file system reusing the current inode number > + * (last_inode). > + */ > + if (last_fd != -1) > + close(last_fd); > last_fd = fd; > last_inode = stb.st_ino; > } > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-11-04 21:51 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-10-28 0:01 [PATCH] Close etab file's file descriptor on stat error Malahal Naineni 2015-10-28 13:53 ` J. Bruce Fields 2015-10-28 16:16 ` Malahal Naineni 2015-11-04 21:51 ` Steve Dickson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox