libnl: recvmsgs doesn't necessarily free the message data

I stepped over libnl always freeing the messages and it
kind of made it awkward to reuse the message data without
reallocating.

The basic idea is: if a callback return value has a bit set,
don't free that message. The calling application owns it.
By default, things stay as before (messages are freed).

Signed-off-by: Inaky Perez-Gonzalez <inaky@linux.intel.com>

diff --git a/include/netlink/handlers.h b/include/netlink/handlers.h
index fcd3e485ecc291a8d8086a87b4f484f6d7db3136..ebcf9117a651baba87a26dc49d958b56b07a98dc 100644 (file)
--- a/include/netlink/handlers.h
+++ b/include/netlink/handlers.h
@@ -64,6 +64,20 @@ enum nl_cb_action {
        NL_STOP,
 };
 
+/**
+ * Callback action modifiers
+ * @ingroup cb
+ *
+ * These should be ORed to the callback actions defined by enum
+ * nl_cb_action.
+ */
+enum nl_cb_action_mods {
+       /** Callee keeps the message, don't free */
+       NL_KEEP_MSG = 0x1000,
+#define NL_KEEP_MSG NL_KEEP_MSG        /* for config testing */
+};
+
+
 /* backwards compatibility */
 #define NL_PROCEED NL_OK
 #define NL_EXIT NL_STOP

diff --git a/lib/nl.c b/lib/nl.c
index b425302b5fbcd910135841cbdb109b81bfde9b62..3281739ba605509440af6874b79ad4f0e4c07654 100644 (file)
--- a/lib/nl.c
+++ b/lib/nl.c
@@ -551,6 +551,9 @@ abort:
 #define NL_CB_CALL(cb, type, msg) \
 do { \
        err = nl_cb_call(cb, type, msg); \
+       if (free_msg && (err & NL_KEEP_MSG))     \
+               free_msg = 0;                    \
+       err &= ~NL_KEEP_MSG;                     \
        switch (err) { \
        case NL_OK: \
                err = 0; \
@@ -564,10 +567,23 @@ do { \
        } \
 } while (0)
 
+/*
+ * NOTE: on handling freeing of the message data
+ *
+ * By default, the message data is freed after handling is done. In
+ * order to allow a callback using it after exiting the message
+ * handling loop, it can return NL_KEEP_MSG ORed to it's return code.
+ * 
+ * Once the freeing of the message is disabled, it cannot be activated
+ * again; this way, if a callback decides to switch it off because it
+ * will keep the allocated data, another one cannot activate it, have
+ * it freed and cause a race condition with later access to that (now
+ * freed) data.
+ */ 
 static int recvmsgs(struct nl_handle *handle, struct nl_cb *cb)
 {
        int n, err = 0, multipart = 0;
-       unsigned char *buf = NULL;
+       unsigned char *buf = NULL, free_msg = 1;
        struct nlmsghdr *hdr;
        struct sockaddr_nl nla = {0};
        struct nl_msg *msg = NULL;
@@ -590,7 +606,9 @@ continue_reading:
                NL_DBG(3, "recgmsgs(%p): Processing valid message...\n",
                       handle);
 
-               nlmsg_free(msg);
+               if (free_msg)
+                       nlmsg_free(msg);
+               free_msg = 1;   /* By default, we free the message data */
                msg = nlmsg_convert(hdr);
                if (!msg) {
                        err = nl_errno(ENOMEM);
@@ -728,7 +746,8 @@ skip:
                hdr = nlmsg_next(hdr, &n);
        }
        
-       nlmsg_free(msg);
+       if (free_msg)
+               nlmsg_free(msg);
        free(buf);
        free(creds);
        buf = NULL;