Sourceware Bugzilla – Attachment 8137 Details for
Bug 14906
inotify failed when /etc/hosts file change
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
glibc-14906.diff
glibc-14906.diff (text/plain), 20.22 KB, created by
Carlos O'Donell
on 2015-02-18 02:12:05 UTC
(
hide
)
Description:
glibc-14906.diff
Filename:
MIME Type:
Creator:
Carlos O'Donell
Created:
2015-02-18 02:12:05 UTC
Size:
20.22 KB
patch
obsolete
>diff --git a/ChangeLog b/ChangeLog >index cdf49aa..c3b53b7 100644 >--- a/ChangeLog >+++ b/ChangeLog >@@ -1,5 +1,27 @@ > 2015-02-17 Carlos O'Donell <carlos@redhat.com> > >+ [BZ #14906] >+ * nscd/cache.c (prune_cache): Use TRACED_FILE. >+ * nscd/connections.c [HAVE_INOTIFY] (install_watches): New function. >+ (register_traced_file): Call install_watches. Don't set unused mtime. >+ (invalidate_cache): Call install_watches. >+ (inotify_check_files): Don't inline. Handle watching parent >+ directories and configuration file movement in and out. >+ (handle_inotify_events): New function. >+ (main_loop_poll): Call handle_inotify_events. >+ (main_loop_epoll): Likewise. >+ * nscd/nscd.h: Define TRACED_FILE, TRACED_DIR, and PATH_MAX. >+ (struct traced_file): Remove unused mtime. Use array of inotify fds. >+ Add parent directory, and basename. >+ (init_traced_file): New inline function. >+ (define_traced_file): New macro. >+ * nss/nss_db/db-init.c: Use define_traced_file. >+ (_nss_db_init): Use init_traced_file. >+ * nss/nss_files/files-init.c: Use define_traced_file. >+ (_nss_files_init): Use init_traced_file. >+ >+2015-02-17 Carlos O'Donell <carlos@redhat.com> >+ > * dl-reloc.c: Inlucde libc-internal.h. > (_dl_try_allocate_static_tls): Call ALIGN_UP. > (_dl_relocate_object): Call ALIGN_UP, ALIGN_DOWN, and PTR_ALIGN_DOWN. >diff --git a/nscd/cache.c b/nscd/cache.c >index ea9f723..926d0b4 100644 >--- a/nscd/cache.c >+++ b/nscd/cache.c >@@ -272,7 +272,7 @@ prune_cache (struct database_dyn *table, time_t now, int fd) > while (runp != NULL) > { > #ifdef HAVE_INOTIFY >- if (runp->inotify_descr == -1) >+ if (runp->inotify_descr[TRACED_FILE] == -1) > #endif > { > struct stat64 st; >diff --git a/nscd/connections.c b/nscd/connections.c >index 985eab6..65ec38e 100644 >--- a/nscd/connections.c >+++ b/nscd/connections.c >@@ -957,6 +957,47 @@ cannot change socket to nonblocking mode: %s"), > finish_drop_privileges (); > } > >+#ifdef HAVE_INOTIFY >+#define TRACED_FILE_MASK (IN_DELETE_SELF | IN_CLOSE_WRITE | IN_MOVE_SELF) >+#define TRACED_DIR_MASK (IN_DELETE_SELF | IN_CREATE | IN_MOVED_TO | IN_MOVE_SELF) >+void >+install_watches (struct traced_file *finfo) >+{ >+ /* If we have inotify support use it exclusively with no fallback >+ to stat. This is a design decision to make the implementation >+ sipmler. Either we use fstat for the file name or we use inotify >+ for both the file and parent directory. */ >+ if (finfo->inotify_descr[TRACED_FILE] < 0) >+ finfo->inotify_descr[TRACED_FILE] = inotify_add_watch (inotify_fd, >+ finfo->fname, >+ TRACED_FILE_MASK); >+ if (finfo->inotify_descr[TRACED_FILE] < 0) >+ { >+ dbg_log (_("failed to add watch to file `%s': %s"), >+ finfo->fname, strerror (errno)); >+ return; >+ } >+ dbg_log (_("watching registered file `%s` (%d)"), >+ finfo->fname, finfo->inotify_descr[TRACED_FILE]); >+ /* Additionally listen for IN_CREATE events in the files parent >+ directory. We do this because the file to be watched might be >+ deleted and then added back again. When it is added back again >+ we must re-add the watch. We must also cover IN_MOVED_TO to >+ detect a file being moved into the directory. */ >+ if (finfo->inotify_descr[TRACED_DIR] < 0) >+ finfo->inotify_descr[TRACED_DIR] = inotify_add_watch (inotify_fd, >+ finfo->dname, >+ TRACED_DIR_MASK); >+ if (finfo->inotify_descr[TRACED_DIR] < 0) >+ { >+ dbg_log (_("failed to add watch to directory `%s': %s"), >+ finfo->fname, strerror (errno)); >+ return; >+ } >+ dbg_log (_("watching registered directory `%s` (%d)"), >+ finfo->dname, finfo->inotify_descr[TRACED_DIR]); >+} >+#endif > > /* Register the file in FINFO as a traced file for the database DBS[DBIX]. > >@@ -985,26 +1026,8 @@ register_traced_file (size_t dbidx, struct traced_file *finfo) > finfo->fname, dbnames[dbidx]); > > #ifdef HAVE_INOTIFY >- if (inotify_fd < 0 >- || (finfo->inotify_descr = inotify_add_watch (inotify_fd, finfo->fname, >- IN_DELETE_SELF >- | IN_MODIFY)) < 0) >+ install_watches (finfo); > #endif >- { >- /* We need the modification date of the file. */ >- struct stat64 st; >- >- if (stat64 (finfo->fname, &st) < 0) >- { >- /* We cannot stat() the file, disable file checking. */ >- dbg_log (_("cannot stat() file `%s': %s"), >- finfo->fname, strerror (errno)); >- return; >- } >- >- finfo->inotify_descr = -1; >- finfo->mtime = st.st_mtime; >- } > > /* Queue up the file name. */ > finfo->next = dbs[dbidx].traced_files; >@@ -1033,13 +1056,20 @@ invalidate_cache (char *key, int fd) > { > struct traced_file *runp = dbs[hstdb].traced_files; > while (runp != NULL) >- if (runp->call_res_init) >- { >- res_init (); >- break; >- } >- else >+ { >+#ifdef HAVE_INOTIFY >+ /* During an invalidation we try to reload the traced >+ file watches. This allows the user to re-sync if >+ inotify events were lost. */ >+ install_watches (runp); >+#endif >+ if (runp->call_res_init) >+ { >+ res_init (); >+ break; >+ } > runp = runp->next; >+ } > } > break; > } >@@ -1884,7 +1914,7 @@ union __inev > registered with a database then mark that database as requiring its cache > to be cleared. We indicate the cache needs clearing by setting > TO_CLEAR[DBCNT] to true for the matching database. */ >-static inline void >+static void > inotify_check_files (bool *to_clear, union __inev *inev) > { > /* Check which of the files changed. */ >@@ -1894,16 +1924,119 @@ inotify_check_files (bool *to_clear, union __inev *inev) > > while (finfo != NULL) > { >- /* Inotify event watch descriptor matches. */ >- if (finfo->inotify_descr == inev->i.wd) >+ /* The configuration file was moved or deleted. >+ We stop watching it at that point, and reinitialize. */ >+ if (finfo->inotify_descr[TRACED_FILE] == inev->i.wd >+ && ((inev->i.mask & IN_MOVE_SELF) >+ || (inev->i.mask & IN_DELETE_SELF) >+ || (inev->i.mask & IN_IGNORED))) >+ { >+ int ret; >+ bool moved = (inev->i.mask & IN_MOVE_SELF) != 0; >+ dbg_log (_("registered file `%s` was %s, removing watch"), >+ finfo->fname, moved ? "moved" : "deleted"); >+ /* File was moved out, remove the watch. Watches are >+ automatically removed when the file is deleted. */ >+ if (moved) >+ { >+ ret = inotify_rm_watch (inotify_fd, inev->i.wd); >+ if (ret < 0) >+ dbg_log (_("failed to remove file watch `%s`: %s"), >+ finfo->fname, strerror (errno)); >+ } >+ finfo->inotify_descr[TRACED_FILE] = -1; >+ to_clear[dbcnt] = true; >+ if (finfo->call_res_init) >+ res_init (); >+ return; >+ } >+ /* The configuration file was open for writing and has just closed. >+ We reset the cache and reinitialize. */ >+ if (finfo->inotify_descr[TRACED_FILE] == inev->i.wd) > { > /* Mark cache as needing to be cleared and reinitialize. */ >+ dbg_log (_("registered file `%s` was written to"), finfo->fname); > to_clear[dbcnt] = true; > if (finfo->call_res_init) > res_init (); > return; > } >+ /* The parent directory was moved or deleted. There is no coming >+ back from this. We do not track the parent of the parent, and >+ once this happens we trigger one last invalidation. You must >+ restart nscd to track subsequent changes. We track this to >+ do one last robust re-initialization and then we're done. */ >+ if (finfo->inotify_descr[TRACED_DIR] == inev->i.wd >+ && ((inev->i.mask & IN_DELETE_SELF) >+ || (inev->i.mask & IN_MOVE_SELF) >+ || (inev->i.mask & IN_IGNORED))) >+ { >+ bool moved = (inev->i.mask & IN_MOVE_SELF) != 0; >+ /* The directory watch may have already been removed >+ but we don't know so we just remove it again and >+ ignore the error. Then we remove the file watch. >+ Note: watches are automatically removed for deleted >+ files. */ >+ if (moved) >+ inotify_rm_watch (inotify_fd, inev->i.wd); >+ if (finfo->inotify_descr[TRACED_FILE] != -1) >+ { >+ dbg_log (_("registered parent directory `%s` was %s, removing watch on `%s`"), >+ finfo->dname, moved ? "moved" : "deleted", finfo->fname); >+ if (inotify_rm_watch (inotify_fd, finfo->inotify_descr[TRACED_FILE]) < 0) >+ dbg_log (_("failed to remove file watch `%s`: %s"), >+ finfo->dname, strerror (errno)); >+ } >+ finfo->inotify_descr[TRACED_FILE] = -1; >+ finfo->inotify_descr[TRACED_DIR] = -1; >+ to_clear[dbcnt] = true; >+ if (finfo->call_res_init) >+ res_init (); >+ /* Continue to the next entry since this might be the >+ parent directory for multiple registered files and >+ we want to remove watches for all registered files. */ >+ continue; >+ } >+ /* The parent directory had a create or moved to event. */ >+ if (finfo->inotify_descr[TRACED_DIR] == inev->i.wd >+ && strcmp (inev->i.name, finfo->sfname) == 0) >+ { >+ /* We detected a directory change. We look for the creation >+ of the file we are tracking or the move of the same file >+ into the directory. */ >+ int ret; >+ dbg_log (_("registered file `%s` was %s, adding watch"), >+ finfo->fname, >+ inev->i.mask & IN_CREATE ? "created" : "moved into place"); >+ /* File was moved in or created. Regenerate the watch. */ >+ if (finfo->inotify_descr[TRACED_FILE] != -1) >+ { >+ ret = inotify_rm_watch (inotify_fd, >+ finfo->inotify_descr[TRACED_FILE]); >+ if (ret < 0) >+ dbg_log (_("failed to remove file watch `%s`: %s"), >+ finfo->fname, strerror (errno)); >+ } >+ >+ ret = inotify_add_watch (inotify_fd, >+ finfo->fname, >+ TRACED_FILE_MASK); >+ if (ret < 0) >+ dbg_log (_("failed to add file watch `%s`: %s"), >+ finfo->fname, strerror (errno)); >+ >+ finfo->inotify_descr[TRACED_FILE] = ret; > >+ /* The file is new or moved so mark cache as needing to >+ be cleared and reinitialize. */ >+ to_clear[dbcnt] = true; >+ if (finfo->call_res_init) >+ res_init (); >+ >+ /* Done re-adding the watch. Don't return, we may still >+ have other files in this same directory, same watch >+ descriptor, and need to process them. */ >+ } > finfo = finfo->next; > } > } >@@ -1925,6 +2058,51 @@ clear_db_cache (bool *to_clear) > } > } > >+int >+handle_inotify_events (void) >+{ >+ bool to_clear[lastdb] = { false, }; >+ union __inev inev; >+ >+ /* Read all inotify events for files registered via >+ register_traced_file(). */ >+ while (1) >+ { >+ /* Potentially read multiple events into buf. */ >+ ssize_t nb = TEMP_FAILURE_RETRY (read (inotify_fd, >+ &inev.buf, >+ sizeof (inev))); >+ if (nb < (ssize_t) sizeof (struct inotify_event)) >+ { >+ /* Not even 1 event. */ >+ if (__glibc_unlikely (nb == -1 && errno != EAGAIN)) >+ return -1; >+ /* Done reading events that are ready. */ >+ break; >+ } >+ /* Process all events. The normal inotify interface delivers >+ complete events on a read and never a partial event. */ >+ char *eptr = &inev.buf[0]; >+ ssize_t count; >+ while (1) >+ { >+ /* Check which of the files changed. */ >+ inotify_check_files (to_clear, &inev); >+ count = sizeof (struct inotify_event) + inev.i.len; >+ eptr += count; >+ nb -= count; >+ if (nb >= (ssize_t) sizeof (struct inotify_event)) >+ memcpy (&inev, eptr, nb); >+ else >+ break; >+ } >+ continue; >+ } >+ /* Actually perform the cache clearing. */ >+ clear_db_cache (to_clear); >+ return 0; >+} >+ > #endif > > static void >@@ -2031,42 +2209,20 @@ main_loop_poll (void) > { > if (conns[1].revents != 0) > { >- bool to_clear[lastdb] = { false, }; >- union __inev inev; >- >- /* Read all inotify events for files registered via >- register_traced_file(). */ >- while (1) >+ int ret; >+ ret = handle_inotify_events (); >+ if (ret == -1) > { >- ssize_t nb = TEMP_FAILURE_RETRY (read (inotify_fd, &inev, >- sizeof (inev))); >- if (nb < (ssize_t) sizeof (struct inotify_event)) >- { >- if (__builtin_expect (nb == -1 && errno != EAGAIN, >- 0)) >- { >- /* Something went wrong when reading the inotify >- data. Better disable inotify. */ >- dbg_log (_("\ >-disabled inotify after read error %d"), >- errno); >- conns[1].fd = -1; >- firstfree = 1; >- if (nused == 2) >- nused = 1; >- close (inotify_fd); >- inotify_fd = -1; >- } >- break; >- } >- >- /* Check which of the files changed. */ >- inotify_check_files (to_clear, &inev); >+ /* Something went wrong when reading the inotify >+ data. Better disable inotify. */ >+ dbg_log (_("disabled inotify after read error %d"), errno); >+ conns[1].fd = -1; >+ firstfree = 1; >+ if (nused == 2) >+ nused = 1; >+ close (inotify_fd); >+ inotify_fd = -1; > } >- >- /* Actually perform the cache clearing. */ >- clear_db_cache (to_clear); >- > --n; > } > >@@ -2234,37 +2390,18 @@ main_loop_epoll (int efd) > # ifdef HAVE_INOTIFY > else if (revs[cnt].data.fd == inotify_fd) > { >- bool to_clear[lastdb] = { false, }; >- union __inev inev; >- >- /* Read all inotify events for files registered via >- register_traced_file(). */ >- while (1) >+ int ret; >+ ret = handle_inotify_events (); >+ if (ret == -1) > { >- ssize_t nb = TEMP_FAILURE_RETRY (read (inotify_fd, &inev, >- sizeof (inev))); >- if (nb < (ssize_t) sizeof (struct inotify_event)) >- { >- if (__glibc_unlikely (nb == -1 && errno != EAGAIN)) >- { >- /* Something went wrong when reading the inotify >- data. Better disable inotify. */ >- dbg_log (_("disabled inotify after read error %d"), >- errno); >- (void) epoll_ctl (efd, EPOLL_CTL_DEL, inotify_fd, >- NULL); >- close (inotify_fd); >- inotify_fd = -1; >- } >- break; >- } >- >- /* Check which of the files changed. */ >- inotify_check_files(to_clear, &inev); >+ /* Something went wrong when reading the inotify >+ data. Better disable inotify. */ >+ dbg_log (_("disabled inotify after read error %d"), errno); >+ (void) epoll_ctl (efd, EPOLL_CTL_DEL, inotify_fd, NULL); >+ close (inotify_fd); >+ inotify_fd = -1; >+ break; > } >- >- /* Actually perform the cache clearing. */ >- clear_db_cache (to_clear); > } > # endif > # ifdef HAVE_NETLINK >diff --git a/nscd/nscd.h b/nscd/nscd.h >index 17a0a96..02e617f 100644 >--- a/nscd/nscd.h >+++ b/nscd/nscd.h >@@ -61,17 +61,64 @@ typedef enum > 80% of the thread stack size. */ > #define MAX_STACK_USE ((8 * NSCD_THREAD_STACKSIZE) / 10) > >- >-/* Registered filename used to fill database. */ >+/* Records the file registered per database that when changed >+ or modified requires invalidating the database. */ > struct traced_file > { >- time_t mtime; >+ /* Support multiple registered files per database. */ > struct traced_file *next; > int call_res_init; >- int inotify_descr; >+ /* Requires Inotify support to do anything useful. */ >+#define TRACED_FILE 0 >+#define TRACED_DIR 1 >+ int inotify_descr[2]; >+# ifndef PATH_MAX >+# define PATH_MAX 1024 >+# endif >+ /* The parent directory is used to scan for creation/deletion. */ >+ char dname[PATH_MAX]; >+ /* Just the name of the file with no directory component. */ >+ char *sfname; >+ /* The full-path name of the registered file. */ > char fname[]; > }; > >+/* Initialize a `struct traced_file`. As input we need the name >+ of the file, and if invalidation requires calling res_init. >+ If CRINIT is 1 then res_init will be called after invalidation >+ or if the traced file is changed in any way, otherwise it will >+ not. */ >+static inline void >+init_traced_file(struct traced_file *file, const char *fname, int crinit) >+{ >+ char *dname; >+ file->inotify_descr[TRACED_FILE] = -1; >+ file->inotify_descr[TRACED_DIR] = -1; >+ strcpy (file->fname, fname); >+ /* Compute the parent directory name and store a copy. The copy makes >+ it much faster to add/remove watches while nscd is running instead >+ of computing this over and over again in a temp buffer. */ >+ file->dname[0] = '\0'; >+ dname = strrchr (fname, '/'); >+ if (dname != NULL) >+ { >+ size_t len = (size_t)(dname - fname); >+ if (len > sizeof (file->dname)) >+ abort (); >+ strncpy (file->dname, file->fname, len); >+ file->dname[len] = '\0'; >+ } >+ /* The basename is the name just after the last forward slash. */ >+ file->sfname = &dname[1]; >+ file->call_res_init = crinit; >+} >+ >+#define define_traced_file(id, filename) \ >+static union \ >+{ \ >+ struct traced_file file; \ >+ char buf[sizeof (struct traced_file) + sizeof (filename)]; \ >+} id##_traced_file; > > /* Structure describing dynamic part of one database. */ > struct database_dyn >diff --git a/nss/nss_db/db-init.c b/nss/nss_db/db-init.c >index 041471c..099d89b 100644 >--- a/nss/nss_db/db-init.c >+++ b/nss/nss_db/db-init.c >@@ -22,35 +22,25 @@ > #include <nscd/nscd.h> > #include <string.h> > >-static union >-{ >- struct traced_file file; >- char buf[sizeof (struct traced_file) + sizeof (_PATH_VARDB "passwd.db")]; >-} pwd_traced_file; >- >-static union >-{ >- struct traced_file file; >- char buf[sizeof (struct traced_file) + sizeof (_PATH_VARDB "group.db")]; >-} grp_traced_file; >+#define PWD_FILENAME (_PATH_VARDB "passwd.db") >+define_traced_file (pwd, PWD_FILENAME); > >-static union >-{ >- struct traced_file file; >- char buf[sizeof (struct traced_file) + sizeof (_PATH_VARDB "services.db")]; >-} serv_traced_file; >+#define GRP_FILENAME (_PATH_VARDB "group.db") >+define_traced_file (grp, GRP_FILENAME); > >+#define SERV_FILENAME (_PATH_VARDB "services.db") >+define_traced_file (serv, SERV_FILENAME); > > void > _nss_db_init (void (*cb) (size_t, struct traced_file *)) > { >- strcpy (pwd_traced_file.file.fname,_PATH_VARDB "passwd.db"); >+ init_traced_file (&pwd_traced_file.file, PWD_FILENAME, 0); > cb (pwddb, &pwd_traced_file.file); > >- strcpy (grp_traced_file.file.fname, _PATH_VARDB "group.db"); >+ init_traced_file (&grp_traced_file.file, GRP_FILENAME, 0); > cb (grpdb, &grp_traced_file.file); > >- strcpy (serv_traced_file.file.fname, _PATH_VARDB "services.db"); >+ init_traced_file (&serv_traced_file.file, SERV_FILENAME, 0); > cb (servdb, &serv_traced_file.file); > } > >diff --git a/nss/nss_files/files-init.c b/nss/nss_files/files-init.c >index 94d440a..72eced4 100644 >--- a/nss/nss_files/files-init.c >+++ b/nss/nss_files/files-init.c >@@ -21,47 +21,43 @@ > #include <string.h> > #include <nscd/nscd.h> > >+#define PWD_FILENAME "/etc/passwd" >+define_traced_file (pwd, PWD_FILENAME); > >-#define TF(id, filename, ...) \ >-static union \ >-{ \ >- struct traced_file file; \ >- char buf[sizeof (struct traced_file) + sizeof (filename)]; \ >-} id##_traced_file = \ >- { \ >- .file = \ >- { \ >- __VA_ARGS__ \ >- } \ >- } >- >-TF (pwd, "/etc/passwd"); >-TF (grp, "/etc/group"); >-TF (hst, "/etc/hosts"); >-TF (resolv, "/etc/resolv.conf", .call_res_init = 1); >-TF (serv, "/etc/services"); >-TF (netgr, "/etc/netgroup"); >+#define GRP_FILENAME "/etc/group" >+define_traced_file (grp, GRP_FILENAME); > >+#define HST_FILENAME "/etc/hosts" >+define_traced_file (hst, HST_FILENAME); >+ >+#define RESOLV_FILENAME "/etc/resolv.conf" >+define_traced_file (resolv, RESOLV_FILENAME); >+ >+#define SERV_FILENAME "/etc/services" >+define_traced_file (serv, SERV_FILENAME); >+ >+#define NETGR_FILENAME "/etc/netgroup" >+define_traced_file (netgr, NETGR_FILENAME); > > void > _nss_files_init (void (*cb) (size_t, struct traced_file *)) > { >- strcpy (pwd_traced_file.file.fname, "/etc/passwd"); >+ init_traced_file (&pwd_traced_file.file, PWD_FILENAME, 0); > cb (pwddb, &pwd_traced_file.file); > >- strcpy (grp_traced_file.file.fname, "/etc/group"); >+ init_traced_file (&grp_traced_file.file, GRP_FILENAME, 0); > cb (grpdb, &grp_traced_file.file); > >- strcpy (hst_traced_file.file.fname, "/etc/hosts"); >+ init_traced_file (&hst_traced_file.file, HST_FILENAME, 0); > cb (hstdb, &hst_traced_file.file); > >- strcpy (resolv_traced_file.file.fname, "/etc/resolv.conf"); >+ init_traced_file (&resolv_traced_file.file, RESOLV_FILENAME, 1); > cb (hstdb, &resolv_traced_file.file); > >- strcpy (serv_traced_file.file.fname, "/etc/services"); >+ init_traced_file (&serv_traced_file.file, SERV_FILENAME, 0); > cb (servdb, &serv_traced_file.file); > >- strcpy (netgr_traced_file.file.fname, "/etc/netgroup"); >+ init_traced_file (&netgr_traced_file.file, NETGR_FILENAME, 0); > cb (netgrdb, &netgr_traced_file.file); > } >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=8137">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
Attachments on
bug 14906
:
6770
|
6771
| 8137