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>

diff --git a/source3/include/proto.h b/source3/include/proto.h
index 0775acd21da28002e67c78232d2b52130c3c8418..91d3ef165e976d3214ff44884b1421e6c99f2ed4 100644 (file)
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -4062,7 +4062,9 @@ void notify_printer_sepfile(struct tevent_context *ev,
 /* The following definitions come from printing/pcap.c  */
 
 void pcap_cache_reload(struct tevent_context *ev,
-                      struct messaging_context *msg_ctx);
+                      struct messaging_context *msg_ctx,
+                      void (*post_cache_fill_fn)(struct tevent_context *,
+                                                 struct messaging_context *));
 bool pcap_printername_ok(const char *printername);
 
 /* The following definitions come from printing/printing.c  */
@@ -5399,7 +5401,8 @@ void server_messaging_context_free(void);
 struct event_context *smbd_event_context(void);
 struct messaging_context *smbd_messaging_context(void);
 struct memcache *smbd_memcache(void);
-void reload_printers(struct messaging_context *msg_ctx);
+void reload_printers(struct tevent_context *ev,
+                    struct messaging_context *msg_ctx);
 bool reload_services(struct messaging_context *msg_ctx, int smb_sock,
                     bool test);
 void exit_server(const char *const explanation);

diff --git a/source3/printing/load.c b/source3/printing/load.c
index 4f1bb88a99636312f381d7a00c0d50ca3d769106..66c3ffd22c0120e9626190f776ac76d9a3145917 100644 (file)
--- a/source3/printing/load.c
+++ b/source3/printing/load.c
@@ -54,14 +54,12 @@ static void add_auto_printers(void)
 }
 
 /***************************************************************************
-load automatic printer services
+load automatic printer services from pre-populated pcap cache
 ***************************************************************************/
 void load_printers(struct tevent_context *ev,
                   struct messaging_context *msg_ctx)
 {
-       if (!pcap_cache_loaded()) {
-               pcap_cache_reload(ev, msg_ctx);
-       }
+       SMB_ASSERT(pcap_cache_loaded());
 
        add_auto_printers();
 

diff --git a/source3/printing/pcap.c b/source3/printing/pcap.c
index 1b8f46d8e719e73170dea443880c394100d9cf4b..be267bd116e560022368ae936402afaec84f2f73 100644 (file)
--- a/source3/printing/pcap.c
+++ b/source3/printing/pcap.c
@@ -107,11 +107,14 @@ void pcap_cache_replace(const struct pcap_cache *pcache)
 }
 
 void pcap_cache_reload(struct tevent_context *ev,
-                      struct messaging_context *msg_ctx)
+                      struct messaging_context *msg_ctx,
+                      void (*post_cache_fill_fn)(struct tevent_context *,
+                                                 struct messaging_context *))
 {
        const char *pcap_name = lp_printcapname();
        bool pcap_reloaded = False;
        NTSTATUS status;
+       bool post_cache_fill_fn_handled = false;
 
        DEBUG(3, ("reloading printcap cache\n"));
 
@@ -135,7 +138,13 @@ void pcap_cache_reload(struct tevent_context *ev,
 
 #ifdef HAVE_CUPS
        if (strequal(pcap_name, "cups")) {
-               pcap_reloaded = cups_cache_reload(ev, msg_ctx);
+               pcap_reloaded = cups_cache_reload(ev, msg_ctx,
+                                                 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
@@ -174,6 +183,10 @@ done:
                if (!NT_STATUS_IS_OK(status)) {
                        DEBUG(0, ("Failed to cleanup printer list!\n"));
                }
+               if ((post_cache_fill_fn_handled == false)
+                && (post_cache_fill_fn != NULL)) {
+                       post_cache_fill_fn(ev, msg_ctx);
+               }
        }
 
        return;

diff --git a/source3/printing/pcap.h b/source3/printing/pcap.h
index 67f36d6598128245a12e9baa3851c25b3dfa01b7..7f8f7d2baf439f47c2d96f000c64773550bea0b1 100644 (file)
--- a/source3/printing/pcap.h
+++ b/source3/printing/pcap.h
@@ -35,7 +35,9 @@ bool aix_cache_reload(void);
 /* The following definitions come from printing/print_cups.c  */
 
 bool cups_cache_reload(struct tevent_context *ev,
-                      struct messaging_context *msg_ctx);
+                      struct messaging_context *msg_ctx,
+                      void (*post_cache_fill_fn)(struct tevent_context *,
+                                                 struct messaging_context *));
 bool cups_pull_comment_location(TALLOC_CTX *mem_ctx,
                                const char *printername,
                                char **comment,

diff --git a/source3/printing/print_cups.c b/source3/printing/print_cups.c
index a85fba89974f316c5e49e33f6d1942b5b87a2d2e..8c023ddb30082447c677f679dd9e430d688044f3 100644 (file)
--- a/source3/printing/print_cups.c
+++ b/source3/printing/print_cups.c
@@ -449,13 +449,22 @@ static bool cups_pcap_load_async(struct tevent_context *ev,
        _exit(0);
 }
 
+struct cups_async_cb_args {
+       int pipe_fd;
+       struct event_context *event_ctx;
+       struct messaging_context *msg_ctx;
+       void (*post_cache_fill_fn)(struct event_context *,
+                                  struct messaging_context *);
+};
+
 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. "
@@ -549,28 +558,43 @@ 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 != NULL) {
+                       cb_args->post_cache_fill_fn(cb_args->event_ctx,
+                                                   cb_args->msg_ctx);
+               }
        } 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(struct tevent_context *ev,
-                      struct messaging_context *msg_ctx)
+                      struct messaging_context *msg_ctx,
+                      void (*post_cache_fill_fn)(struct tevent_context *,
+                                                 struct messaging_context *))
 {
-       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 == NULL) {
                return false;
        }
 
+       cb_args->post_cache_fill_fn = post_cache_fill_fn;
+       cb_args->event_ctx = ev;
+       cb_args->msg_ctx = msg_ctx;
+       p_pipe_fd = &cb_args->pipe_fd;
        *p_pipe_fd = -1;
 
        /* Set up an async refresh. */
        if (!cups_pcap_load_async(ev, msg_ctx, p_pipe_fd)) {
+               talloc_free(cb_args);
                return false;
        }
        if (!local_pcap_copy) {
@@ -582,7 +606,7 @@ bool cups_cache_reload(struct tevent_context *ev,
 
                cups_async_callback(ev, NULL,
                                        EVENT_FD_READ,
-                                       (void *)p_pipe_fd);
+                                       (void *)cb_args);
                if (!local_pcap_copy) {
                        return false;
                }
@@ -599,10 +623,10 @@ bool cups_cache_reload(struct tevent_context *ev,
                                        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/process.c b/source3/smbd/process.c
index 150b2dd2cb1fbeaac177a8fc698f574109163ad5..2992576702ad60d39c735ebe74bd993c9b4c8f20 100644 (file)
--- a/source3/smbd/process.c
+++ b/source3/smbd/process.c
@@ -2259,7 +2259,8 @@ static void check_reload(struct smbd_server_connection *sconn, time_t t)
                        || (t-last_printer_reload_time  < 0) ) 
                {
                        DEBUG( 3,( "Printcap cache time expired.\n"));
-                       reload_printers(sconn->msg_ctx);
+                       pcap_cache_reload(server_event_context(),
+                                         sconn->msg_ctx, &reload_printers);
                        last_printer_reload_time = t;
                }
        }

diff --git a/source3/smbd/server.c b/source3/smbd/server.c
index 1b9e793e4c5eab493761ba524a868ed38c3d6ebb..16aa055b3531decd7f46c08ea6f429310b14296b 100644 (file)
--- a/source3/smbd/server.c
+++ b/source3/smbd/server.c
@@ -1238,7 +1238,8 @@ extern void build_options(bool screen);
        }
 
        /* Publish nt printers, this requires a working winreg pipe */
-       reload_printers(smbd_messaging_context());
+       pcap_cache_reload(server_event_context(), smbd_messaging_context(),
+                         &reload_printers);
 
        /* only start the background queue daemon if we are 
           running as a daemon -- bad things will happen if

diff --git a/source3/smbd/server_reload.c b/source3/smbd/server_reload.c
index 38d1f3a354acb75c76135741e892b0fcd29e25d3..2b74e7a0d49379a3146029d776a6dcc3ddb405c3 100644 (file)
--- a/source3/smbd/server_reload.c
+++ b/source3/smbd/server_reload.c
@@ -25,11 +25,13 @@
 #include "smbd/globals.h"
 #include "librpc/gen_ndr/messaging.h"
 #include "nt_printing.h"
+#include "printing/pcap.h"
 
 /****************************************************************************
- Reload printers
+ purge stale printers and reload from pre-populated pcap cache
 **************************************************************************/
-void reload_printers(struct messaging_context *msg_ctx)
+void reload_printers(struct tevent_context *ev,
+                    struct messaging_context *msg_ctx)
 {
        struct auth_serversupplied_info *server_info = NULL;
        struct spoolss_PrinterInfo2 *pinfo2 = NULL;
@@ -40,7 +42,8 @@ void reload_printers(struct messaging_context *msg_ctx)
        NTSTATUS status;
        bool skip = false;
 
-       pcap_cache_reload(server_event_context(), msg_ctx);
+       SMB_ASSERT(pcap_cache_loaded());
+       DEBUG(10, ("reloading printer services from pcap cache\n"));
 
        status = make_server_info_system(talloc_tos(), &server_info);
        if (!NT_STATUS_IS_OK(status)) {
@@ -79,7 +82,7 @@ void reload_printers(struct messaging_context *msg_ctx)
                }
        }
 
-       load_printers(server_event_context(), msg_ctx);
+       load_printers(ev, msg_ctx);
 
        TALLOC_FREE(server_info);
 }
@@ -111,7 +114,7 @@ bool reload_services(struct messaging_context *msg_ctx, int smb_sock,
 
        ret = lp_load(get_dyn_CONFIGFILE(), False, False, True, True);
 
-       reload_printers(msg_ctx);
+       pcap_cache_reload(server_event_context(), msg_ctx, &reload_printers);
 
        /* perhaps the config filename is now set */
        if (!test)

diff --git a/source3/web/swat.c b/source3/web/swat.c
index bb3f3f974c6d3bd36915401e7e5c53660b852764..1cbecd4675a3a2fe169faa076d88ee9b1ae5897e 100644 (file)
--- a/source3/web/swat.c
+++ b/source3/web/swat.c
@@ -491,7 +491,8 @@ static int save_reload(int snum)
                 return 0;
         }
        iNumNonAutoPrintServices = lp_numservices();
-       load_printers(server_event_context(), server_messaging_context());
+       pcap_cache_reload(server_event_context(), server_messaging_context(),
+                         &load_printers);
 
        return 1;
 }
@@ -1435,7 +1436,8 @@ const char *lang_msg_rotate(TALLOC_CTX *ctx, const char *msgid)
        reopen_logs();
        load_interfaces();
        iNumNonAutoPrintServices = lp_numservices();
-       load_printers(server_event_context(), server_messaging_context());
+       pcap_cache_reload(server_event_context(), server_messaging_context(),
+                         &load_printers);
 
        cgi_setup(get_dyn_SWATDIR(), !demo_mode);