Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix socket can race condition #223

Merged
merged 7 commits into from
Mar 13, 2024
Merged

Fix socket can race condition #223

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
26ac367
Revert "net/can, net/devif: fix CAN RX/TX iob free semcount runaway i…
jlaitine Mar 8, 2024
97d95ce
net/can: Release IOB when failed to queue in datahandler
wengzhe Dec 27, 2023
bbe2a43
net/can/can_callback.c: Take the timestamp size into account in "buflen"
haitomatic Mar 12, 2024
ba3ff71
net/devif/devif_send.c: Don't update the pktlen to 0
haitomatic Mar 12, 2024
5e92321
net/can/can_recvmsg.c: Remove IOB trimming as useless, can frames can…
haitomatic Mar 12, 2024
55fb40f
arch/risc-v/src/mpfs/mpfs_fpga_canfd.c: Fix a race condition with dev…
jlaitine Mar 12, 2024
4231379
net/can/can_recvmsg.c: Cleanup can_readahead
jlaitine Mar 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions arch/risc-v/src/mpfs/mpfs_fpga_canfd.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,12 @@ static void mpfs_receive_work(void *arg)

mpfs_can_retrieve_rx_frame(priv, cf, ffw);

/* Lock the network; we have to protect the dev.d_len, dev.d_buf
* and dev.d_iob from the devif_poll path
*/

net_lock();

/* Copy the buffer pointer to priv->dev.. Set amount of data
* in priv->dev.d_len
*/
Expand All @@ -714,6 +720,8 @@ static void mpfs_receive_work(void *arg)
NETDEV_RXPACKETS(&priv->dev);
can_input(&priv->dev);

net_unlock();

/* Point the packet buffer back to the next Tx buffer that will be
* used during the next write. If the write queue is full, then
* this will point at an active buffer, which must not be written
Expand Down
29 changes: 11 additions & 18 deletions net/can/can_callback.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,12 @@ can_data_event(FAR struct net_driver_s *dev, FAR struct can_conn_s *conn,
uint16_t flags)
{
int buflen = dev->d_len;
uint16_t recvlen;
uint16_t ret;

#ifdef CONFIG_NET_TIMESTAMP
buflen -= sizeof(struct timeval);
#endif
uint16_t recvlen;
uint16_t ret;

ret = (flags & ~CAN_NEWDATA);

Expand Down Expand Up @@ -136,19 +137,6 @@ uint16_t can_callback(FAR struct net_driver_s *dev,

if ((flags & CAN_NEWDATA) != 0)
{
if (dev->d_iob->io_flink != NULL ||
dev->d_iob->io_pktlen == 0 ||
dev->d_iob->io_offset <= 0)
{
if (dev->d_iob->io_flink == NULL)
{
iob_free(dev->d_iob);
}

netdev_iob_clear(dev);
return flags;
}

#ifdef CONFIG_NET_TIMESTAMP
/* TIMESTAMP sockopt is activated,
* create timestamp and copy to iob
Expand Down Expand Up @@ -230,11 +218,16 @@ uint16_t can_datahandler(FAR struct net_driver_s *dev,
can_readahead_signal(conn);
#endif
ret = iob->io_pktlen;
}

/* Device buffer must be enqueue or freed, clear the handle */
/* Device buffer has been enqueued, clear the handle */

netdev_iob_clear(dev);
netdev_iob_clear(dev);
}
else
{
nerr("ERROR: Failed to queue the I/O buffer chain: %d\n", ret);
netdev_iob_release(dev);
}

return ret;
}
Expand Down
50 changes: 7 additions & 43 deletions net/can/can_recvmsg.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,22 +233,9 @@ static inline int can_readahead(struct can_recvfrom_s *pstate)

pstate->pr_recvlen = -1;

if ((iob = iob_peek_queue(&conn->readahead)) != NULL &&
if ((iob = iob_remove_queue(&conn->readahead)) != NULL &&
pstate->pr_buflen > 0)
{
if (iob->io_flink != NULL ||
iob->io_pktlen == 0 ||
iob->io_offset <= 0)
{
if (iob->io_pktlen == 0 || iob->io_offset <= 0)
{
iob_free(iob);
}

iob_remove_queue(&conn->readahead);
return 0;
}

DEBUGASSERT(iob->io_pktlen > 0);

#ifdef CONFIG_NET_CANPROTO_OPTIONS
Expand All @@ -259,17 +246,7 @@ static inline int can_readahead(struct can_recvfrom_s *pstate)

if (can_recv_filter(conn, can_id) == 0)
{
FAR struct iob_s *tmp;

/* Remove the I/O buffer chain from the head of the read-ahead
* buffer queue.
*/

tmp = iob_remove_queue(&conn->readahead);
DEBUGASSERT(tmp == iob);
UNUSED(tmp);

/* And free the I/O buffer chain */
/* Free the I/O buffer chain */

iob_free_chain(iob);
return 0;
Expand All @@ -290,27 +267,14 @@ static inline int can_readahead(struct can_recvfrom_s *pstate)

recvlen = iob_copyout(pstate->pr_buffer, iob, pstate->pr_buflen, 0);

/* If we took all of the data from the I/O buffer chain is empty, then
* release it. If there is still data available in the I/O buffer
* chain, then just trim the data that we have taken from the
* beginning of the I/O buffer chain.
*/

/* No trimming needed since one CAN/CANFD frame can perfectly
* fit in one iob
*/

FAR struct iob_s *tmp;

/* Remove the I/O buffer chain from the head of the read-ahead
* buffer queue.
/* We should have taken all of the data from the I/O buffer chain,
* so release it. There is no trimming needed, since One CAN/CANFD
* frame can always fit in one IOB.
*/

tmp = iob_remove_queue(&conn->readahead);
DEBUGASSERT(tmp == iob);
UNUSED(tmp);
static_assert(sizeof(struct can_frame) <= CONFIG_IOB_BUFSIZE);

/* And free the I/O buffer chain */
/* Free the I/O buffer chain */

iob_free_chain(iob);

Expand Down
Loading