casaroli commented on code in PR #21517:
URL: https://github.com/apache/nuttx/pull/21517#discussion_r4501776757
##########
net/pkt/pkt_recvmsg.c:
##########
@@ -465,7 +506,7 @@ ssize_t pkt_recvmsg(FAR struct socket *psock, FAR struct
msghdr *msg,
* enough to hold this address family.
*/
- if (from != NULL && *fromlen < sizeof(sa_family_t))
+ if (from != NULL && *fromlen < sizeof(struct sockaddr_ll))
Review Comment:
A `fromlen` smaller than `sizeof(struct sockaddr_ll)` now fails with
`EINVAL`; before, it was accepted. POSIX says that a long address is truncated,
so `recvfrom()` with a `struct sockaddr` buffer must still work.
Fix: keep the old check, fill a local `struct sockaddr_ll`, and copy at most
`*fromlen` bytes of it.
##########
net/pkt/pkt_recvmsg.c:
##########
@@ -154,9 +156,33 @@ static void pkt_recvfrom_newdata(FAR struct net_driver_s
*dev,
*
****************************************************************************/
-static inline void pkt_recvfrom_sender(FAR struct net_driver_s *dev,
+static inline void pkt_recvfrom_sender(int ifindex, FAR struct iob_s *iob,
FAR struct pkt_recvfrom_s *pstate)
{
+ FAR struct sockaddr_ll *from;
+
+ if (pstate->pr_msg->msg_name != NULL)
+ {
+ from = (FAR struct sockaddr_ll *)pstate->pr_msg->msg_name;
+ from->sll_family = AF_PACKET;
+ from->sll_ifindex = ifindex;
+ from->sll_protocol = pstate->pr_type;
+
+ from->sll_halen = ETH_ALEN;
+
+ if (iob != NULL && iob->io_pktlen >= sizeof(struct eth_hdr_s))
+ {
+ FAR struct eth_hdr_s *ethhdr =
+ (FAR struct eth_hdr_s *)iob->io_data;
Review Comment:
`iob->io_data` is the start of the IOB buffer, not the Ethernet header. For
`dev->d_iob` (line 244) the header starts at `IOB_DATA(iob) -
NET_LL_HDRLEN(dev)`, so `sll_addr` gets other bytes: a frame from
00:e0:de:ad:be:ef returned 00:00:00:00:00:00 on qemu-armv8a.
Fix: pass the start of the L2 header: `dev->d_buf` (set to `NETLLBUF` in
`pkt_in_()`) from the event handler and `IOB_DATA(iob)` from `pkt_readdata()`.
##########
net/pkt/pkt_recvmsg.c:
##########
@@ -411,6 +439,19 @@ static inline int pkt_readdata(FAR struct pkt_recvfrom_s
*pstate,
tsfunc(pstate, iob);
+ /* Fill in the sender's address information if requested */
+
+ if (dev != NULL && pstate->pr_msg->msg_name != NULL)
+ {
+ pkt_recvfrom_sender(dev->d_ifindex, iob, pstate);
+ }
+
+ /* Remove the I/O buffer chain from the head of the read-ahead
+ * buffer queue.
+ */
+
+ iob_remove_queue(&conn->readahead);
Review Comment:
`pkt_readdata()` already took this IOB off the queue with
`iob_remove_queue(iobq)` at line 409, so this call removes the next queued
frame and never frees it. With 4 frames queued, `recvfrom()` returns frames 1
and 3 only, and the IOBs of frames 2 and 4 are never freed (`/proc/iobinfo` on
qemu-armv8a). With `MSG_ERRQUEUE`, `iobq` is `errahead`, so this drops a data
frame.
Fix: remove this call.
--
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]
