[ovs-dev] [PATCH v3 05/11] datapath-windows/Netlink: Fixed NlAttrParseNested

Ankur Sharma ankursharma at vmware.com
Sat Sep 27 00:32:49 UTC 2014


NlAttrParseNested was using the whole netlink payload for iteration.
This is not correct, as it would lead to exceeding the
nested attribute boundries. Fixed the same in this patch.

Signed-off-by: Ankur Sharma <ankursharma at vmware.com>
Acked-by: Alin Gabriel Serdean <aserdean at cloudbasesolutions.com>
Acked-by: Eitan Eliahu <eliahue at vmware.com>
Acked-by: Nithin Raju <nithin at vmware.com>
Acked-by: Samuel Ghinet <sghinet at cloudbasesolutions.com>
Tested-by: Ankur Sharma <ankursharma at vmware.com>

---
 datapath-windows/ovsext/Datapath.c        |  4 +++-
 datapath-windows/ovsext/Netlink/Netlink.c | 25 +++++++++++++++++--------
 datapath-windows/ovsext/Netlink/Netlink.h | 14 +++++++-------
 3 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-windows/ovsext/Datapath.c
index 0dfdd57..2b7c6ea 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -949,7 +949,8 @@ OvsSubscribeEventCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
         (POVS_OPEN_INSTANCE)usrParamsCtx->ovsInstance;
     POVS_MESSAGE msgIn = (POVS_MESSAGE)usrParamsCtx->inputBuffer;
 
-    rc = NlAttrParse(&msgIn->nlMsg, sizeof (*msgIn),policy, attrs, 2);
+    rc = NlAttrParse(&msgIn->nlMsg, sizeof (*msgIn),
+         NlMsgAttrsLen((PNL_MSG_HDR)msgIn), policy, attrs, 2);
     if (!rc) {
         status = STATUS_INVALID_PARAMETER;
         goto done;
@@ -1107,6 +1108,7 @@ HandleDpTransaction(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
     if (usrParamsCtx->ovsMsg->genlMsg.cmd == OVS_DP_CMD_SET) {
         if (!NlAttrParse((PNL_MSG_HDR)msgIn,
                         NLMSG_HDRLEN + GENL_HDRLEN + OVS_HDRLEN,
+                        NlMsgAttrsLen((PNL_MSG_HDR)msgIn),
                         ovsDatapathSetPolicy, dpAttrs, ARRAY_SIZE(dpAttrs))) {
             return STATUS_INVALID_PARAMETER;
         }
diff --git a/datapath-windows/ovsext/Netlink/Netlink.c b/datapath-windows/ovsext/Netlink/Netlink.c
index ce95623..0c4f847 100644
--- a/datapath-windows/ovsext/Netlink/Netlink.c
+++ b/datapath-windows/ovsext/Netlink/Netlink.c
@@ -558,7 +558,7 @@ NlMsgSize(const PNL_MSG_HDR nlh)
  * ---------------------------------------------------------------------------
  */
 PCHAR
-NlMsgPayload(const PNL_MSG_HDR nlh)
+NlHdrPayload(const PNL_MSG_HDR nlh)
 {
     return ((PCHAR)nlh + NLMSG_HDRLEN);
 }
@@ -569,7 +569,7 @@ NlMsgPayload(const PNL_MSG_HDR nlh)
  * ---------------------------------------------------------------------------
  */
 UINT32
-NlMsgPayloadLen(const PNL_MSG_HDR nlh)
+NlHdrPayloadLen(const PNL_MSG_HDR nlh)
 {
     return nlh->nlmsgLen - NLMSG_HDRLEN;
 }
@@ -582,7 +582,7 @@ NlMsgPayloadLen(const PNL_MSG_HDR nlh)
 PNL_ATTR
 NlMsgAttrs(const PNL_MSG_HDR nlh)
 {
-    return (PNL_ATTR) (NlMsgPayload(nlh) + GENL_HDRLEN + OVS_HDRLEN);
+    return (PNL_ATTR) (NlHdrPayload(nlh) + GENL_HDRLEN + OVS_HDRLEN);
 }
 
 /*
@@ -591,9 +591,9 @@ NlMsgAttrs(const PNL_MSG_HDR nlh)
  * ---------------------------------------------------------------------------
  */
 UINT32
-NlMsgAttrLen(const PNL_MSG_HDR nlh)
+NlMsgAttrsLen(const PNL_MSG_HDR nlh)
 {
-    return NlMsgPayloadLen(nlh) - GENL_HDRLEN - OVS_HDRLEN;
+    return NlHdrPayloadLen(nlh) - GENL_HDRLEN - OVS_HDRLEN;
 }
 
 /* Netlink message parse. */
@@ -974,6 +974,7 @@ NlAttrFindNested(const PNL_ATTR nla, UINT16 type)
  */
 BOOLEAN
 NlAttrParse(const PNL_MSG_HDR nlMsg, UINT32 attrOffset,
+            UINT32 totalAttrLen,
             const NL_POLICY policy[],
             PNL_ATTR attrs[], UINT32 n_attrs)
 {
@@ -984,14 +985,21 @@ NlAttrParse(const PNL_MSG_HDR nlMsg, UINT32 attrOffset,
 
     RtlZeroMemory(attrs, n_attrs * sizeof *attrs);
 
-    if ((NlMsgSize(nlMsg) < attrOffset) || (!(NlMsgAttrLen(nlMsg)))) {
+
+    /* There is nothing to parse */
+    if (!(NlMsgAttrsLen(nlMsg))) {
+        ret = TRUE;
+        goto done;
+    }
+
+    if ((NlMsgSize(nlMsg) < attrOffset)) {
         OVS_LOG_WARN("No attributes in nlMsg: %p at offset: %d",
                      nlMsg, attrOffset);
         goto done;
     }
 
     NL_ATTR_FOR_EACH (nla, left, NlMsgAt(nlMsg, attrOffset),
-                      NlMsgSize(nlMsg) - attrOffset)
+                      totalAttrLen)
     {
         UINT16 type = NlAttrType(nla);
         if (type < n_attrs && policy[type].type != NL_A_NO_ATTR) {
@@ -1040,9 +1048,10 @@ done:
  */
 BOOLEAN
 NlAttrParseNested(const PNL_MSG_HDR nlMsg, UINT32 attrOffset,
+                  UINT32 totalAttrLen,
                   const NL_POLICY policy[],
                   PNL_ATTR attrs[], UINT32 n_attrs)
 {
     return NlAttrParse(nlMsg, attrOffset + NLA_HDRLEN,
-                       policy, attrs, n_attrs);
+                       totalAttrLen - NLA_HDRLEN, policy, attrs, n_attrs);
 }
diff --git a/datapath-windows/ovsext/Netlink/Netlink.h b/datapath-windows/ovsext/Netlink/Netlink.h
index 13304e8..26772b7 100644
--- a/datapath-windows/ovsext/Netlink/Netlink.h
+++ b/datapath-windows/ovsext/Netlink/Netlink.h
@@ -86,10 +86,10 @@ NTSTATUS NlFillOvsMsg(PNL_BUFFER nlBuf,
 /* Netlink message accessing the payload */
 PVOID NlMsgAt(const PNL_MSG_HDR nlh, UINT32 offset);
 UINT32 NlMsgSize(const PNL_MSG_HDR nlh);
-PCHAR NlMsgPayload(const PNL_MSG_HDR nlh);
-UINT32 NlMsgPayloadLen(const PNL_MSG_HDR nlh);
+PCHAR NlHdrPayload(const PNL_MSG_HDR nlh);
+UINT32 NlHdrPayloadLen(const PNL_MSG_HDR nlh);
 PNL_ATTR NlMsgAttrs(const PNL_MSG_HDR nlh);
-UINT32 NlMsgAttrLen(const PNL_MSG_HDR nlh);
+UINT32 NlMsgAttrsLen(const PNL_MSG_HDR nlh);
 
 /* Netlink message parse */
 PNL_MSG_HDR NlMsgNext(const PNL_MSG_HDR nlh);
@@ -116,11 +116,11 @@ const PNL_ATTR NlAttrFind__(const PNL_ATTR attrs,
 const PNL_ATTR NlAttrFindNested(const PNL_ATTR nla,
                                 UINT16 type);
 BOOLEAN NlAttrParse(const PNL_MSG_HDR nlMsg, UINT32 attrOffset,
-                    const NL_POLICY policy[],
+                    UINT32 totalAttrLen, const NL_POLICY policy[],
                     PNL_ATTR attrs[], UINT32 n_attrs);
-BOOLEAN NlParseNested(const PNL_ATTR, const NL_POLICY policy[],
-                      PNL_ATTR attrs[], UINT32 n_attrs);
-
+BOOLEAN NlAttrParseNested(const PNL_MSG_HDR nlMsg, UINT32 attrOffset,
+                          UINT32 totalAttrLen, const NL_POLICY policy[],
+                          PNL_ATTR attrs[], UINT32 n_attrs);
 /*
  * --------------------------------------------------------------------------
  * Returns the length of attribute.
-- 
1.9.1




More information about the dev mailing list