casaroli commented on code in PR #20627:
URL: https://github.com/apache/nuttx/pull/20627#discussion_r4392214823
##########
net/tcp/tcp_input.c:
##########
@@ -1343,6 +1559,31 @@ static void tcp_input(FAR struct net_driver_s *dev,
uint8_t domain,
/* Queue out-of-order segments. */
tcp_input_ofosegs(dev, conn, iplen);
+#endif
+#ifdef CONFIG_NET_TCPURGDATA
+ /* RFC 793: If the URG bit is set in an out-of-order segment,
+ * announce the urgent pointer and notify listeners.
+ */
+
+ if ((tcp->flags & TCP_URG) != 0)
Review Comment:
`tcp_input_ofosegs()` at line 1561 can free `dev->d_iob`, and `tcp` points
into it, so reading `tcp->flags` and `tcp->urgp` here is a use after free.
Fix: read the URG flag and `urgp` into locals before that call.
##########
net/tcp/tcp_input.c:
##########
@@ -1649,15 +1853,34 @@ static void tcp_input(FAR struct net_driver_s *dev,
uint8_t domain,
* send, d_len must be set to 0.
*/
+#ifdef CONFIG_NET_TCPURGDATA
+ if ((flags & (TCP_NEWDATA | TCP_ACKDATA | TCP_URGDATA)) != 0)
+#else
if ((flags & (TCP_NEWDATA | TCP_ACKDATA)) != 0)
+#endif
{
dev->d_sndlen = 0;
/* Provide the packet to the application */
result = tcp_callback(dev, conn, flags);
- /* Send the response, ACKing the data or not, as appropriate */
+ /* Send the response, ACKing the data or not, as appropriate.
+ * If urgent data was received, ensure an ACK is sent even if
+ * d_len was 0.
+ */
+
+#ifdef CONFIG_NET_TCPURGDATA
+ if (urg_removed && dev->d_len == 0)
Review Comment:
`rcvseq` gets its +1 only after `tcp_callback()`, which already compares it
with the out-of-order queue, so the next queued segment waits for a
retransmission.
Fix: add the 1 in `tcp_urg_input()` in both cases and remove it here.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]