Re: [ovs-dev] [PATCH v1 05/10] datapath-windows/Netlink: Fixed NlAttrParseNested
Hi Sam,
Please find my replies inline.
Regards,
Ankur
From: Samuel Ghinet sghi...@cloudbasesolutions.com
Sent: Thursday, September 25, 2014 8:30 AM
To: dev@openvswitch.org
Cc: Ankur Sharma
Subject: RE: [ovs-dev] [PATCH v1 05/10] datapath-windows/Netlink: Fixed
NlAttrParseNested
Hello Ankur,
This code confused me a little.
It looks correct, but I would have a few suggestions:
o)
BOOLEAN
NlAttrParse(const PNL_MSG_HDR nlMsg, UINT32 attrOffset,
UINT32 attrLen,
the attrLen field gives the impression that it is the length of one single
attribute.
You could try attributesLen, or totalAttrLen or smth similar, perhaps.
[ANKUR]: done
o) I find the functions NlMsgAttrLen and NlMsgPayloadLen a bit confusing:
[CODE]
UINT32
NlMsgAttrLen(const PNL_MSG_HDR nlh)
{
return NlMsgPayloadLen(nlh) - GENL_HDRLEN - OVS_HDRLEN;
}
[/CODE]
NlMsgPayloadLen: when you hear payload, you think about the buffer, without
NL_MSG_HDR, HDRLEN and HDRLEN.
NlMsgAttrLen: I tended to read it like Netlink Message Attribute Length, and
was thinking, it computes the length of the first netlink attribute? Perhaps a
name like OvsMsgPayloadLength() would be more intuitive.
[ANKUR]: I have renamed the methods for more clarity.
NlMsgPayload NlHdrPayload
NlMsgPayloadLen NlHdrPayloadLen
NlMsgAttrLen == NlMsgAttrsLen
o) I find it a bit odd that a function like NlAttrParse, which parses
attributes, would require: the message header, the ovs message payload length
and the offset.
Perhaps it would have been simpler if NlAttrParse had assumed that the pointer
provided is the beginning of the first netlink attribute, and not validate the
message itself.
It is possible such changes would make code clearer a bit.
However, there is existing code that relies on the current format of
NlAttrParse, so perhaps on a new patch.
[ANKUR]: Point noted. I have a few more netlink related series coming in next
week. This comment will be addressed there.
Acked-by: Samuel Ghinet sghi...@cloudbasesolutions.com
Date: Wed, 24 Sep 2014 00:15:39 -0700
From: Ankur Sharma ankursha...@vmware.com
To: dev@openvswitch.org
Subject: [ovs-dev] [PATCH v1 05/10] datapath-windows/Netlink: Fixed
NlAttrParseNested
Message-ID: 1411542944-19374-5-git-send-email-ankursha...@vmware.com
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.
---
datapath-windows/ovsext/Datapath.c| 4 +++-
datapath-windows/ovsext/Netlink/Netlink.c | 15 ---
datapath-windows/ovsext/Netlink/Netlink.h | 8
3 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/datapath-windows/ovsext/Datapath.c
b/datapath-windows/ovsext/Datapath.c
index 0dfdd57..ffb7d44 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),
+ NlMsgAttrLen((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,
+NlMsgAttrLen((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 5c74ec0..a72d846 100644
--- a/datapath-windows/ovsext/Netlink/Netlink.c
+++ b/datapath-windows/ovsext/Netlink/Netlink.c
@@ -969,6 +969,7 @@ NlAttrFindNested(const PNL_ATTR nla, UINT16 type)
*/
BOOLEAN
NlAttrParse(const PNL_MSG_HDR nlMsg, UINT32 attrOffset,
+UINT32 attrLen,
const NL_POLICY policy[],
PNL_ATTR attrs[], UINT32 n_attrs)
{
@@ -979,14 +980,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 (!(NlMsgAttrLen(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
Re: [ovs-dev] [PATCH v1 05/10] datapath-windows/Netlink: Fixed NlAttrParseNested
Hello Ankur,
This code confused me a little.
It looks correct, but I would have a few suggestions:
o)
BOOLEAN
NlAttrParse(const PNL_MSG_HDR nlMsg, UINT32 attrOffset,
UINT32 attrLen,
the attrLen field gives the impression that it is the length of one single
attribute.
You could try attributesLen, or totalAttrLen or smth similar, perhaps.
o) I find the functions NlMsgAttrLen and NlMsgPayloadLen a bit confusing:
[CODE]
UINT32
NlMsgAttrLen(const PNL_MSG_HDR nlh)
{
return NlMsgPayloadLen(nlh) - GENL_HDRLEN - OVS_HDRLEN;
}
[/CODE]
NlMsgPayloadLen: when you hear payload, you think about the buffer, without
NL_MSG_HDR, HDRLEN and HDRLEN.
NlMsgAttrLen: I tended to read it like Netlink Message Attribute Length, and
was thinking, it computes the length of the first netlink attribute? Perhaps a
name like OvsMsgPayloadLength() would be more intuitive.
o) I find it a bit odd that a function like NlAttrParse, which parses
attributes, would require: the message header, the ovs message payload length
and the offset.
Perhaps it would have been simpler if NlAttrParse had assumed that the pointer
provided is the beginning of the first netlink attribute, and not validate the
message itself.
It is possible such changes would make code clearer a bit.
However, there is existing code that relies on the current format of
NlAttrParse, so perhaps on a new patch.
Acked-by: Samuel Ghinet sghi...@cloudbasesolutions.com
Date: Wed, 24 Sep 2014 00:15:39 -0700
From: Ankur Sharma ankursha...@vmware.com
To: dev@openvswitch.org
Subject: [ovs-dev] [PATCH v1 05/10] datapath-windows/Netlink: Fixed
NlAttrParseNested
Message-ID: 1411542944-19374-5-git-send-email-ankursha...@vmware.com
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.
---
datapath-windows/ovsext/Datapath.c| 4 +++-
datapath-windows/ovsext/Netlink/Netlink.c | 15 ---
datapath-windows/ovsext/Netlink/Netlink.h | 8
3 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/datapath-windows/ovsext/Datapath.c
b/datapath-windows/ovsext/Datapath.c
index 0dfdd57..ffb7d44 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),
+ NlMsgAttrLen((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,
+NlMsgAttrLen((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 5c74ec0..a72d846 100644
--- a/datapath-windows/ovsext/Netlink/Netlink.c
+++ b/datapath-windows/ovsext/Netlink/Netlink.c
@@ -969,6 +969,7 @@ NlAttrFindNested(const PNL_ATTR nla, UINT16 type)
*/
BOOLEAN
NlAttrParse(const PNL_MSG_HDR nlMsg, UINT32 attrOffset,
+UINT32 attrLen,
const NL_POLICY policy[],
PNL_ATTR attrs[], UINT32 n_attrs)
{
@@ -979,14 +980,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 (!(NlMsgAttrLen(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)
+ attrLen)
{
UINT16 type = NlAttrType(nla);
if (type n_attrs policy[type].type != NL_A_NO_ATTR) {
@@ -1035,9 +1043,10 @@ done:
*/
BOOLEAN
NlAttrParseNested(const PNL_MSG_HDR nlMsg, UINT32 attrOffset,
+ UINT32 attrLen,
const NL_POLICY policy[],
PNL_ATTR attrs[], UINT32 n_attrs)
{
return NlAttrParse(nlMsg, attrOffset + NLA_HDRLEN,
- policy, attrs, n_attrs);
+ attrLen - NLA_HDRLEN, policy, attrs, n_attrs);
}
diff --git a/datapath-windows
[ovs-dev] [PATCH v1 05/10] datapath-windows/Netlink: Fixed NlAttrParseNested
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.
---
datapath-windows/ovsext/Datapath.c| 4 +++-
datapath-windows/ovsext/Netlink/Netlink.c | 15 ---
datapath-windows/ovsext/Netlink/Netlink.h | 8
3 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/datapath-windows/ovsext/Datapath.c
b/datapath-windows/ovsext/Datapath.c
index 0dfdd57..ffb7d44 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),
+ NlMsgAttrLen((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,
+NlMsgAttrLen((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 5c74ec0..a72d846 100644
--- a/datapath-windows/ovsext/Netlink/Netlink.c
+++ b/datapath-windows/ovsext/Netlink/Netlink.c
@@ -969,6 +969,7 @@ NlAttrFindNested(const PNL_ATTR nla, UINT16 type)
*/
BOOLEAN
NlAttrParse(const PNL_MSG_HDR nlMsg, UINT32 attrOffset,
+UINT32 attrLen,
const NL_POLICY policy[],
PNL_ATTR attrs[], UINT32 n_attrs)
{
@@ -979,14 +980,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 (!(NlMsgAttrLen(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)
+ attrLen)
{
UINT16 type = NlAttrType(nla);
if (type n_attrs policy[type].type != NL_A_NO_ATTR) {
@@ -1035,9 +1043,10 @@ done:
*/
BOOLEAN
NlAttrParseNested(const PNL_MSG_HDR nlMsg, UINT32 attrOffset,
+ UINT32 attrLen,
const NL_POLICY policy[],
PNL_ATTR attrs[], UINT32 n_attrs)
{
return NlAttrParse(nlMsg, attrOffset + NLA_HDRLEN,
- policy, attrs, n_attrs);
+ attrLen - NLA_HDRLEN, policy, attrs, n_attrs);
}
diff --git a/datapath-windows/ovsext/Netlink/Netlink.h
b/datapath-windows/ovsext/Netlink/Netlink.h
index 80f98dd..023c673 100644
--- a/datapath-windows/ovsext/Netlink/Netlink.h
+++ b/datapath-windows/ovsext/Netlink/Netlink.h
@@ -125,11 +125,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 attrLen, 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 attrLen, const NL_POLICY policy[],
+ PNL_ATTR attrs[], UINT32 n_attrs);
/*
* --
* Returns the length of attribute.
--
1.9.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v1 05/10] datapath-windows/Netlink: Fixed NlAttrParseNested
Acked-by: Eitan Eliahu elia...@vmware.com
Thanks,
Eitan
-Original Message-
From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Ankur Sharma
Sent: Wednesday, September 24, 2014 12:16 AM
To: dev@openvswitch.org
Subject: [ovs-dev] [PATCH v1 05/10] datapath-windows/Netlink: Fixed
NlAttrParseNested
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.
---
datapath-windows/ovsext/Datapath.c| 4 +++-
datapath-windows/ovsext/Netlink/Netlink.c | 15 ---
datapath-windows/ovsext/Netlink/Netlink.h | 8
3 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/datapath-windows/ovsext/Datapath.c
b/datapath-windows/ovsext/Datapath.c
index 0dfdd57..ffb7d44 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),
+ NlMsgAttrLen((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,
+NlMsgAttrLen((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 5c74ec0..a72d846 100644
--- a/datapath-windows/ovsext/Netlink/Netlink.c
+++ b/datapath-windows/ovsext/Netlink/Netlink.c
@@ -969,6 +969,7 @@ NlAttrFindNested(const PNL_ATTR nla, UINT16 type)
*/
BOOLEAN
NlAttrParse(const PNL_MSG_HDR nlMsg, UINT32 attrOffset,
+UINT32 attrLen,
const NL_POLICY policy[],
PNL_ATTR attrs[], UINT32 n_attrs) { @@ -979,14 +980,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 (!(NlMsgAttrLen(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)
+ attrLen)
{
UINT16 type = NlAttrType(nla);
if (type n_attrs policy[type].type != NL_A_NO_ATTR) { @@ -1035,9
+1043,10 @@ done:
*/
BOOLEAN
NlAttrParseNested(const PNL_MSG_HDR nlMsg, UINT32 attrOffset,
+ UINT32 attrLen,
const NL_POLICY policy[],
PNL_ATTR attrs[], UINT32 n_attrs) {
return NlAttrParse(nlMsg, attrOffset + NLA_HDRLEN,
- policy, attrs, n_attrs);
+ attrLen - NLA_HDRLEN, policy, attrs, n_attrs);
}
diff --git a/datapath-windows/ovsext/Netlink/Netlink.h
b/datapath-windows/ovsext/Netlink/Netlink.h
index 80f98dd..023c673 100644
--- a/datapath-windows/ovsext/Netlink/Netlink.h
+++ b/datapath-windows/ovsext/Netlink/Netlink.h
@@ -125,11 +125,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 attrLen, 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 attrLen, const NL_POLICY policy[],
+ PNL_ATTR attrs[], UINT32 n_attrs);
/*
* --
* Returns the length of attribute.
--
1.9.1
___
dev mailing list
dev@openvswitch.org
https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/listinfo/devk=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0Ar=yTvML8OxA42Jb6ViHe7fUXbvPVOYDPVq87w43doxtlY%3D%0Am=kkhAIamLtKij%2F5cg3nTqp9DNP7g8AQbwAIF16x2xKww%3D%0As=8f452a3de83689b6e7d37378b289ee20afd91635eb95862ea96687aa3ec3db9b
Re: [ovs-dev] [PATCH v1 05/10] datapath-windows/Netlink: Fixed NlAttrParseNested
Acked-by: Alin Gabriel Serdean aserd...@cloudbasesolutions.com
-Mesaj original-
De la: dev [mailto:dev-boun...@openvswitch.org] În numele Ankur Sharma
Trimis: Wednesday, September 24, 2014 10:16 AM
Către: dev@openvswitch.org
Subiect: [ovs-dev] [PATCH v1 05/10] datapath-windows/Netlink: Fixed
NlAttrParseNested
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.
---
datapath-windows/ovsext/Datapath.c| 4 +++-
datapath-windows/ovsext/Netlink/Netlink.c | 15 ---
datapath-windows/ovsext/Netlink/Netlink.h | 8
3 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/datapath-windows/ovsext/Datapath.c
b/datapath-windows/ovsext/Datapath.c
index 0dfdd57..ffb7d44 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),
+ NlMsgAttrLen((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,
+NlMsgAttrLen((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 5c74ec0..a72d846 100644
--- a/datapath-windows/ovsext/Netlink/Netlink.c
+++ b/datapath-windows/ovsext/Netlink/Netlink.c
@@ -969,6 +969,7 @@ NlAttrFindNested(const PNL_ATTR nla, UINT16 type)
*/
BOOLEAN
NlAttrParse(const PNL_MSG_HDR nlMsg, UINT32 attrOffset,
+UINT32 attrLen,
const NL_POLICY policy[],
PNL_ATTR attrs[], UINT32 n_attrs) { @@ -979,14 +980,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 (!(NlMsgAttrLen(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)
+ attrLen)
{
UINT16 type = NlAttrType(nla);
if (type n_attrs policy[type].type != NL_A_NO_ATTR) { @@ -1035,9
+1043,10 @@ done:
*/
BOOLEAN
NlAttrParseNested(const PNL_MSG_HDR nlMsg, UINT32 attrOffset,
+ UINT32 attrLen,
const NL_POLICY policy[],
PNL_ATTR attrs[], UINT32 n_attrs) {
return NlAttrParse(nlMsg, attrOffset + NLA_HDRLEN,
- policy, attrs, n_attrs);
+ attrLen - NLA_HDRLEN, policy, attrs, n_attrs);
}
diff --git a/datapath-windows/ovsext/Netlink/Netlink.h
b/datapath-windows/ovsext/Netlink/Netlink.h
index 80f98dd..023c673 100644
--- a/datapath-windows/ovsext/Netlink/Netlink.h
+++ b/datapath-windows/ovsext/Netlink/Netlink.h
@@ -125,11 +125,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 attrLen, 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 attrLen, const NL_POLICY policy[],
+ PNL_ATTR attrs[], UINT32 n_attrs);
/*
* --
* Returns the length of attribute.
--
1.9.1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
