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 <[email protected]> Acked-by: Alin Gabriel Serdean <[email protected]> Acked-by: Eitan Eliahu <[email protected]> Acked-by: Nithin Raju <[email protected]> Acked-by: Samuel Ghinet <[email protected]> Tested-by: Ankur Sharma <[email protected]> --- 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 _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
