s3-printing: reload shares after pcap cache fill

Since commit eada8f8a, updates to the cups pcap cache are performed
asynchronously - cups_cache_reload() forks a child process to request
cups printer information and notify the parent smbd on completion.

Currently printer shares are reloaded immediately following the call to
cups_cache_reload(), this occurs prior to smbd receiving new cups pcap
information from the child process. Such behaviour can result in stale
print shares as outlined in bug 7836.

This fix ensures print shares are only reloaded after new pcap data has
been received.

Pair-Programmed-With: Lars Müller <lars@samba.org>
(cherry picked from commit a8a01e4a3dcafd97372021d0d6f859fd3a69235f)

diff --git a/source3/include/proto.h b/source3/include/proto.h
index bfcd0122582ed8ea28ece6ee97e9b3abb2703503..8250e54bec026688abd73480dce4e3befa6bd52d 100644 (file)
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -4897,7 +4897,7 @@ void pcap_cache_destroy_specific(struct pcap_cache **ppcache);
 bool pcap_cache_add(const char *name, const char *comment);
 bool pcap_cache_loaded(void);
 void pcap_cache_replace(const struct pcap_cache *cache);
-void pcap_cache_reload(void);
+void pcap_cache_reload(void (*post_cache_fill_fn)(void));
 bool pcap_printername_ok(const char *printername);
 void pcap_printer_fn_specific(const struct pcap_cache *, void (*fn)(const char *, const char *, void *), void *);
 void pcap_printer_fn(void (*fn)(const char *, const char *, void *), void *);
@@ -4908,7 +4908,7 @@ bool aix_cache_reload(void);
 
 /* The following definitions come from printing/print_cups.c  */
 
-bool cups_cache_reload(void);
+bool cups_cache_reload(void (*post_cache_fill_fn)(void));
 bool cups_pull_comment_location(NT_PRINTER_INFO_LEVEL_2 *printer);
 
 /* The following definitions come from printing/print_generic.c  */

diff --git a/source3/printing/load.c b/source3/printing/load.c
index 874f7f25215560a5717905baed1f5810fea4e208..00da9cb2921d350bc2ec9b24fe68ec3a96978d3e 100644 (file)
--- a/source3/printing/load.c
+++ b/source3/printing/load.c
@@ -53,12 +53,11 @@ static void add_auto_printers(void)
 }
 
 /***************************************************************************
-load automatic printer services
+load automatic printer services from pre-populated pcap cache
 ***************************************************************************/
 void load_printers(void)
 {
-       if (!pcap_cache_loaded())
-               pcap_cache_reload();
+       SMB_ASSERT(pcap_cache_loaded());
 
        add_auto_printers();
 

diff --git a/source3/printing/pcap.c b/source3/printing/pcap.c
index a6bf52a0a4ced7559cfa66520f1543ceac82fd88..0d6480ce0159a89e9349be8cafd3a0137de13e54 100644 (file)
--- a/source3/printing/pcap.c
+++ b/source3/printing/pcap.c
@@ -125,13 +125,14 @@ void pcap_cache_replace(const struct pcap_cache *pcache)
        }
 }
 
-void pcap_cache_reload(void)
+void pcap_cache_reload(void (*post_cache_fill_fn)(void))
 {
        const char *pcap_name = lp_printcapname();
        bool pcap_reloaded = False;
        struct pcap_cache *tmp_cache = NULL;
        XFILE *pcap_file;
        char *pcap_line;
+       bool post_cache_fill_fn_handled = false;
 
        DEBUG(3, ("reloading printcap cache\n"));
 
@@ -146,7 +147,12 @@ void pcap_cache_reload(void)
 
 #ifdef HAVE_CUPS
        if (strequal(pcap_name, "cups")) {
-               pcap_reloaded = cups_cache_reload();
+               pcap_reloaded = cups_cache_reload(post_cache_fill_fn);
+               /*
+                * cups_cache_reload() is async and calls post_cache_fill_fn()
+                * on successful completion
+                */
+               post_cache_fill_fn_handled = true;
                goto done;
        }
 #endif
@@ -242,9 +248,13 @@ void pcap_cache_reload(void)
 done:
        DEBUG(3, ("reload status: %s\n", (pcap_reloaded) ? "ok" : "error"));
 
-       if (pcap_reloaded)
+       if (pcap_reloaded) {
                pcap_cache_destroy_specific(&tmp_cache);
-       else {
+               if ((post_cache_fill_fn_handled == false)
+                && (post_cache_fill_fn != NULL)) {
+                       post_cache_fill_fn();
+               }
+       } else {
                pcap_cache_destroy_specific(&pcap_cache);
                pcap_cache = tmp_cache;
        }

diff --git a/source3/printing/print_cups.c b/source3/printing/print_cups.c
index 4c24e4465aef58fe7860613e0b641586564b7013..82b431427ff7ba681868eea0045a8442b54cecb0 100644 (file)
--- a/source3/printing/print_cups.c
+++ b/source3/printing/print_cups.c
@@ -445,13 +445,19 @@ static bool cups_pcap_load_async(int *pfd)
        _exit(0);
 }
 
+struct cups_async_cb_args {
+       int pipe_fd;
+       void (*post_cache_fill_fn)(void);
+};
+
 static void cups_async_callback(struct event_context *event_ctx,
                                struct fd_event *event,
                                uint16 flags,
                                void *p)
 {
        TALLOC_CTX *frame = talloc_stackframe();
-       int fd = *(int *)p;
+       struct cups_async_cb_args *cb_args = (struct cups_async_cb_args *)p;
+       int fd = cb_args->pipe_fd;
        struct pcap_cache *tmp_pcap_cache = NULL;
 
        DEBUG(5,("cups_async_callback: callback received for printer data. "
@@ -545,27 +551,36 @@ static void cups_async_callback(struct event_context *event_ctx,
 
                /* And the systemwide pcap cache. */
                pcap_cache_replace(local_pcap_copy);
+
+               /* Caller may have requested post cache fill callback */
+               if (cb_args->post_cache_fill_fn) {
+                       cb_args->post_cache_fill_fn();
+               }
        } else {
                DEBUG(2,("cups_async_callback: failed to read a new "
                        "printer list\n"));
        }
        close(fd);
-       TALLOC_FREE(p);
+       TALLOC_FREE(cb_args);
        TALLOC_FREE(cache_fd_event);
 }
 
-bool cups_cache_reload(void)
+bool cups_cache_reload(void (*post_cache_fill_fn)(void))
 {
-       int *p_pipe_fd = TALLOC_P(NULL, int);
+       struct cups_async_cb_args *cb_args;
+       int *p_pipe_fd;
 
-       if (!p_pipe_fd) {
+       cb_args = TALLOC_P(NULL, struct cups_async_cb_args);
+       if (!cb_args) {
                return false;
        }
-
+       cb_args->post_cache_fill_fn = post_cache_fill_fn;
+       p_pipe_fd = &cb_args->pipe_fd;
        *p_pipe_fd = -1;
 
        /* Set up an async refresh. */
        if (!cups_pcap_load_async(p_pipe_fd)) {
+               talloc_free(cb_args);
                return false;
        }
        if (!local_pcap_copy) {
@@ -578,7 +593,7 @@ bool cups_cache_reload(void)
                cups_async_callback(smbd_event_context(),
                                        NULL,
                                        EVENT_FD_READ,
-                                       (void *)p_pipe_fd);
+                                       (void *)cb_args);
                if (!local_pcap_copy) {
                        return false;
                }
@@ -595,10 +610,10 @@ bool cups_cache_reload(void)
                                        NULL, *p_pipe_fd,
                                        EVENT_FD_READ,
                                        cups_async_callback,
-                                       (void *)p_pipe_fd);
+                                       (void *)cb_args);
                if (!cache_fd_event) {
                        close(*p_pipe_fd);
-                       TALLOC_FREE(p_pipe_fd);
+                       talloc_free(cb_args);
                        return false;
                }
        }

diff --git a/source3/smbd/server.c b/source3/smbd/server.c
index 0c591357548a9e0a1c4096310c24bc348a59d663..901b8164cc9d3c3e0e1df6c44bf09f5c2396e3a6 100644 (file)
--- a/source3/smbd/server.c
+++ b/source3/smbd/server.c
@@ -734,9 +734,9 @@ static void smbd_parent_loop(struct smbd_parent_context *parent)
 /* NOTREACHED  return True; */
 }
 
-/****************************************************************************
- Reload printers
-**************************************************************************/
+/***************************************************************************
+ purge stale printers and reload from pre-populated pcap cache
+***************************************************************************/
 void reload_printers(void)
 {
        int snum;
@@ -744,9 +744,9 @@ void reload_printers(void)
        int pnum = lp_servicenumber(PRINTERS_NAME);
        const char *pname;
 
-       pcap_cache_reload();
+       SMB_ASSERT(pcap_cache_loaded());
 
-       /* remove stale printers */
+       DEBUG(10, ("reloading printer services from pcap cache\n"));
        for (snum = 0; snum < n_services; snum++) {
                /* avoid removing PRINTERS_NAME or non-autoloaded printers */
                if (snum == pnum || !(lp_snum_ok(snum) && lp_print_ok(snum) &&
@@ -793,7 +793,7 @@ bool reload_services(bool test)
 
        ret = lp_load(get_dyn_CONFIGFILE(), False, False, True, True);
 
-       reload_printers();
+       pcap_cache_reload(&reload_printers);
 
        /* perhaps the config filename is now set */
        if (!test)

diff --git a/source3/web/swat.c b/source3/web/swat.c
index 230b1610c756403cbccd80c4923fa244b8a21de6..baffa44bae1b145272e30a1af8adeb824b2705b5 100644 (file)
--- a/source3/web/swat.c
+++ b/source3/web/swat.c
@@ -490,7 +490,7 @@ static int save_reload(int snum)
                 return 0;
         }
        iNumNonAutoPrintServices = lp_numservices();
-       load_printers();
+       pcap_cache_reload(&load_printers);
 
        return 1;
 }
@@ -1434,7 +1434,7 @@ const char *lang_msg_rotate(TALLOC_CTX *ctx, const char *msgid)
        load_config(True);
        load_interfaces();
        iNumNonAutoPrintServices = lp_numservices();
-       load_printers();
+       pcap_cache_reload(&load_printers);
 
        cgi_setup(get_dyn_SWATDIR(), !demo_mode);