[GitHub] [incubator-nuttx] zhhyu7 commented on a diff in pull request #6702: tcp: check option length before d_len update

[GitBox <gi...@apache.org>]

zhhyu7 commented on code in PR #6702:
URL: https://github.com/apache/incubator-nuttx/pull/6702#discussion_r928861268


##########
net/tcp/tcp_input.c:
##########
@@ -616,6 +616,17 @@ static void tcp_input(FAR struct net_driver_s *dev, uint8_t domain,
       goto drop;
     }
 
+  /* d_appdata should remove the tcp specific option field. */
+
+  if ((tcp->tcpoffset & 0xf0) > 0x50)
+    {
+      len = ((tcp->tcpoffset >> 4) - 5) << 2;

Review Comment:
   > I do not understand this. The `len` value is unconditionally overwritten few lines below. Maybe `len = (tcp->tcpoffset >> 4) << 2;` at line 634 should be relocated to the `else` of `if ((tcp->tcpoffset & 0xf0) > 0x50)`?
   
   They're calculated separately, so maybe they're a little redundant. Offset do not contain options length when tcp_ipvx_select, so need to skip options field when TCP_NEWDATA flag set. Should I skip options field in tcp_ipvx_select function with IPvxTCP_HDRLEN? It doesn't feel right either.



-- 
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