You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/09/05 14:22:47 UTC
[GitHub] [incubator-nuttx-apps] XinStellaris opened a new pull request, #1307: netutils/dhcpc:Make DHCP xid random
XinStellaris opened a new pull request, #1307:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1307
Signed-off-by: 田昕 <ti...@xiaomi.com>
## Summary
In this patch, dhcp will try to get a random xid. If that fails, dhcp will use the default xid.
The reason to do so is from RFC2131:
"The 'xid' field is used by the client to match incoming DHCP messages
with pending requests. A DHCP client MUST choose 'xid's in such a
way as to minimize the chance of using an 'xid' identical to one used
by another client. For example, a client may choose a different,
random initial 'xid' each time the client is rebooted, and
subsequently use sequential 'xid's until the next reboot. Selecting
a new 'xid' for each retransmission is an implementation decision. A
client may choose to reuse the same 'xid' or select a new 'xid' for
each retransmitted message."
## Impact
DHCP
## Testing
BL602 tested
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] XinStellaris commented on pull request #1307: netutils/dhcpc:Make DHCP xid random
Posted by GitBox <gi...@apache.org>.
XinStellaris commented on PR #1307:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1307#issuecomment-1237198338
I think we should put it in dhcp request, so that if we keep the dhcpc thread, xid will change every time just like the rfc requested.
---Original---
From: "Xiang ***@***.***>
Date: Mon, Sep 5, 2022 22:56 PM
To: ***@***.***>;
Cc: ***@***.******@***.***>;
Subject: Re: [apache/incubator-nuttx-apps] netutils/dhcpc:Make DHCP xid random(PR #1307)
@xiaoxiang781216 commented on this pull request.
In netutils/dhcpc/dhcpc.c:
> @@ -145,7 +146,7 @@ struct dhcpc_state_s * Private Data ****************************************************************************/ -static const uint8_t xid[4] = +static uint8_t xid[4] =
let's move xid to the context
In netutils/dhcpc/dhcpc.c:
> @@ -665,6 +666,17 @@ int dhcpc_request(FAR void *handle, FAR struct dhcpc_state *presult) int retries; int state; + /* RFC2131: A DHCP client MUST choose 'xid's in such a + * way as to minimize the chance of using an 'xid' identical to one used + * by another client. + */ + + ret = getrandom(&xid, sizeof(xid), 0);
move to dhcpc_open
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you authored the thread.Message ID: ***@***.***>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 merged pull request #1307: netutils/dhcpc:Make DHCP xid random
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #1307:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1307
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a diff in pull request #1307: netutils/dhcpc:Make DHCP xid random
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #1307:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1307#discussion_r962985951
##########
netutils/dhcpc/dhcpc.c:
##########
@@ -665,6 +666,17 @@ int dhcpc_request(FAR void *handle, FAR struct dhcpc_state *presult)
int retries;
int state;
+ /* RFC2131: A DHCP client MUST choose 'xid's in such a
+ * way as to minimize the chance of using an 'xid' identical to one used
+ * by another client.
+ */
+
+ ret = getrandom(&xid, sizeof(xid), 0);
Review Comment:
move to dhcpc_open
##########
netutils/dhcpc/dhcpc.c:
##########
@@ -145,7 +146,7 @@ struct dhcpc_state_s
* Private Data
****************************************************************************/
-static const uint8_t xid[4] =
+static uint8_t xid[4] =
Review Comment:
let's move xid to the context
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] XinStellaris commented on pull request #1307: netutils/dhcpc:Make DHCP xid random
Posted by GitBox <gi...@apache.org>.
XinStellaris commented on PR #1307:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1307#issuecomment-1237623713
@anchao please review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] XinStellaris commented on pull request #1307: netutils/dhcpc:Make DHCP xid random
Posted by GitBox <gi...@apache.org>.
XinStellaris commented on PR #1307:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1307#issuecomment-1237623202
> Please check whether dhcp6 need the similar change.
Currently there is no similar change in dhcp6. If there is, I will change that in another patch
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a diff in pull request #1307: netutils/dhcpc:Make DHCP xid random
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #1307:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1307#discussion_r963020555
##########
netutils/dhcpc/dhcpc.c:
##########
@@ -665,6 +666,17 @@ int dhcpc_request(FAR void *handle, FAR struct dhcpc_state *presult)
int retries;
int state;
+ /* RFC2131: A DHCP client MUST choose 'xid's in such a
+ * way as to minimize the chance of using an 'xid' identical to one used
+ * by another client.
+ */
+
+ ret = getrandom(&xid, sizeof(xid), 0);
Review Comment:
here is my suggestion:
1.Move xid to dhcpc_state_s
2.Initialize xid with random number in dhcp_open
3.update inside dhcp_request
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on pull request #1307: netutils/dhcpc:Make DHCP xid random
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #1307:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1307#issuecomment-1237167126
Please check whether dhcp6 need the similar change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] XinStellaris commented on a diff in pull request #1307: netutils/dhcpc:Make DHCP xid random
Posted by GitBox <gi...@apache.org>.
XinStellaris commented on code in PR #1307:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1307#discussion_r963006957
##########
netutils/dhcpc/dhcpc.c:
##########
@@ -665,6 +666,17 @@ int dhcpc_request(FAR void *handle, FAR struct dhcpc_state *presult)
int retries;
int state;
+ /* RFC2131: A DHCP client MUST choose 'xid's in such a
+ * way as to minimize the chance of using an 'xid' identical to one used
+ * by another client.
+ */
+
+ ret = getrandom(&xid, sizeof(xid), 0);
Review Comment:
I think we should put it in dhcp request, so that if we keep the dhcpc thread, xid will change every time just like the rfc requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] XinStellaris commented on a diff in pull request #1307: netutils/dhcpc:Make DHCP xid random
Posted by GitBox <gi...@apache.org>.
XinStellaris commented on code in PR #1307:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1307#discussion_r963481746
##########
netutils/dhcpc/dhcpc.c:
##########
@@ -145,7 +147,7 @@ struct dhcpc_state_s
* Private Data
****************************************************************************/
-static const uint8_t xid[4] =
Review Comment:
Sure
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] anchao commented on a diff in pull request #1307: netutils/dhcpc:Make DHCP xid random
Posted by GitBox <gi...@apache.org>.
anchao commented on code in PR #1307:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1307#discussion_r963438859
##########
netutils/dhcpc/dhcpc.c:
##########
@@ -145,7 +147,7 @@ struct dhcpc_state_s
* Private Data
****************************************************************************/
-static const uint8_t xid[4] =
Review Comment:
how about move the global array to corresponding function to save the rodata?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a diff in pull request #1307: netutils/dhcpc:Make DHCP xid random
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #1307:
URL: https://github.com/apache/incubator-nuttx-apps/pull/1307#discussion_r963229987
##########
netutils/dhcpc/dhcpc.c:
##########
@@ -665,6 +680,17 @@ int dhcpc_request(FAR void *handle, FAR struct dhcpc_state *presult)
int retries;
int state;
+ /* RFC2131: A DHCP client MUST choose 'xid's in such a
+ * way as to minimize the chance of using an 'xid' identical to one used
+ * by another client.
+ */
+
+ result = getrandom(pdhcpc->xid, 4, 0);
Review Comment:
let's increase xid instead
##########
netutils/dhcpc/dhcpc.c:
##########
@@ -517,6 +520,18 @@ FAR void *dhcpc_open(FAR const char *interface, FAR const void *macaddr,
pdhcpc = malloc(sizeof(struct dhcpc_state_s) + maclen - 1);
if (pdhcpc)
{
+ /* RFC2131: A DHCP client MUST choose 'xid's in such a
+ * way as to minimize the chance of using an 'xid' identical to one
+ * used by another client.
+ */
+
+ memcpy(pdhcpc->xid, default_xid, 4);
+ result = getrandom(pdhcpc->xid, 4, 0);
+ if (result < 0)
+ {
+ getrandom(pdhcpc->xid, 4, GRND_RANDOM);
Review Comment:
let's move memcpy to last:
```
result = getrandom(pdhcpc->xid, 4, GRND_RANDOM);
if (result != 4)
{
memcpy(pdhcpc->xid, default_xid, 4);
}
```
##########
netutils/dhcpc/dhcpc.c:
##########
@@ -517,6 +520,18 @@ FAR void *dhcpc_open(FAR const char *interface, FAR const void *macaddr,
pdhcpc = malloc(sizeof(struct dhcpc_state_s) + maclen - 1);
if (pdhcpc)
{
+ /* RFC2131: A DHCP client MUST choose 'xid's in such a
+ * way as to minimize the chance of using an 'xid' identical to one
+ * used by another client.
+ */
+
+ memcpy(pdhcpc->xid, default_xid, 4);
+ result = getrandom(pdhcpc->xid, 4, 0);
Review Comment:
reuse ret
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org