From 95c7e943e3ffa9717fed6237a5f5a12409fea43a Mon Sep 17 00:00:00 2001 From: dongjiuzhu1 Date: Fri, 19 Apr 2024 12:29:15 +0800 Subject: [PATCH 01/10] drivers/sesnros: fix crash because ipc buffer pointer used after free When the ipc buffer is obtained for the first time due to insufficient space, it waits due to insufficient ipc buffer. At this time, if rptun recursively operates the next ipc request, the ipc buffer will be reused, but it has been released at this time. 0 file_read (filep=filep@entry=0x20596738, buf=buf@entry=0x205d4358, nbytes=1560) at vfs/fs_read.c:86 1 sensor_rpmsg_push_event_one (dev=0x20558f70, dev@entry=0x2058fc80, stub=stub@entry=0x20596720) at sensors/sensor_rpmsg.c:799 2 sensor_rpmsg_alloc_stub (dev=dev@entry=0x2058fc80, ept=ept@entry=0x20558f08, cookie=) at sensors/sensor_rpmsg.c:552 3 sensor_rpmsg_sub_handler (ept=0x20558f08, data=0x204849c0, len=51, src=, priv=0x20558f00) at sensors/sensor_rpmsg.c:993 4 sensor_rpmsg_ept_cb (ept=, data=, len=, src=, priv=0x20558f00) at sensors/sensor_rpmsg.c:1186 5 rpmsg_virtio_rx_callback (vq=) at open-amp/lib/rpmsg/rpmsg_virtio.c:605 6 virtqueue_notification (vq=) at open-amp/lib/virtio/virtqueue.c:711 7 rproc_virtio_notified (vdev=vdev@entry=0x20558c98, notifyid=notifyid@entry=4294967295) at open-amp/lib/remoteproc/remoteproc_virtio.c:433 8 remoteproc_get_notification (rproc=rproc@entry=0x2054ff34, notifyid=notifyid@entry=4294967295) at open-amp/lib/remoteproc/remoteproc.c:1002 9 rptun_worker (arg=0x2054ff30) at rptun/rptun.c:339 10 rptun_notify_wait (rproc=, id=) at rptun/rptun.c:543 11 remoteproc_virtio_notify_wait (priv=, id=) at open-amp/lib/remoteproc/remoteproc.c:907 12 rproc_virtio_notify_wait (vdev=, vq=) at open-amp/lib/remoteproc/remoteproc_virtio.c:176 13 rpmsg_virtio_notify_wait (vq=, rvdev=0x2054ff78) at nuttx/include/openamp/rpmsg_virtio.h:162 14 rpmsg_virtio_get_tx_payload_buffer (rdev=0x2054ff78, len=0x20558f90, wait=) at open-amp/lib/rpmsg/rpmsg_virtio.c:404 15 rpmsg_get_tx_payload_buffer (ept=ept@entry=0x20558f08, len=len@entry=0x20558f90, wait=wait@entry=1) at open-amp/lib/rpmsg/rpmsg.c:207 16 sensor_rpmsg_push_event_one (dev=0x0, dev@entry=0x20590d60, stub=stub@entry=0x20596720) at sensors/sensor_rpmsg.c:783 17 sensor_rpmsg_alloc_stub (dev=dev@entry=0x20590d60, ept=ept@entry=0x20558f08, cookie=) at sensors/sensor_rpmsg.c:552 18 sensor_rpmsg_sub_handler (ept=0x20558f08, data=0x20483700, len=49, src=, priv=0x20558f00) at sensors/sensor_rpmsg.c:993 19 sensor_rpmsg_ept_cb (ept=, data=, len=, src=, priv=0x20558f00) at sensors/sensor_rpmsg.c:1186 20 rpmsg_virtio_rx_callback (vq=) at open-amp/lib/rpmsg/rpmsg_virtio.c:605 21 virtqueue_notification (vq=) at open-amp/lib/virtio/virtqueue.c:711 22 rproc_virtio_notified (vdev=vdev@entry=0x20558c98, notifyid=notifyid@entry=4294967295) at open-amp/lib/remoteproc/remoteproc_virtio.c:433 23 remoteproc_get_notification (rproc=0x2054ff34, rproc, notifyid=notifyid@entry=4294967295) 24 rptun_worker (arg=0x2054ff30) at rptun/rptun.c:339 25 rptun_thread (argc=, argv=) at rptun/rptun.c:375 Signed-off-by: dongjiuzhu1 --- drivers/sensors/sensor_rpmsg.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/sensors/sensor_rpmsg.c b/drivers/sensors/sensor_rpmsg.c index 6cf1476f15eb6..2fb359b3d56e0 100644 --- a/drivers/sensors/sensor_rpmsg.c +++ b/drivers/sensors/sensor_rpmsg.c @@ -833,6 +833,7 @@ static void sensor_rpmsg_push_event_one(FAR struct sensor_rpmsg_dev_s *dev, if (sre->buffer) { rpmsg_send_nocopy(&sre->ept, sre->buffer, sre->written); + sre->buffer = NULL; } msg = rpmsg_get_tx_payload_buffer(&sre->ept, &sre->space, true); From 5e3443060e51d2168679f46c31db71321167959f Mon Sep 17 00:00:00 2001 From: dongjiuzhu1 Date: Tue, 30 Apr 2024 10:01:30 +0800 Subject: [PATCH 02/10] drivers/sensors: release rpmsg resource when register_driver failed Signed-off-by: dongjiuzhu1 --- drivers/sensors/sensor.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/sensors/sensor.c b/drivers/sensors/sensor.c index 9a42611a0283d..7717b9af3caa0 100644 --- a/drivers/sensors/sensor.c +++ b/drivers/sensors/sensor.c @@ -1329,7 +1329,7 @@ int sensor_custom_register(FAR struct sensor_lowerhalf_s *lower, if (lower == NULL) { ret = -EIO; - goto drv_err; + goto rpmsg_err; } #endif @@ -1345,6 +1345,11 @@ int sensor_custom_register(FAR struct sensor_lowerhalf_s *lower, return ret; drv_err: +#ifdef CONFIG_SENSORS_RPMSG + sensor_rpmsg_unregister(lower); +rpmsg_err: +#endif + nxrmutex_destroy(&upper->lock); kmm_free(upper); From 461670b81b2414e1c3c7c5c33b92403dfe2deb21 Mon Sep 17 00:00:00 2001 From: dongjiuzhu1 Date: Fri, 10 May 2024 12:38:07 +0800 Subject: [PATCH 03/10] drivers/sensors: fix minor issue about sensor driver Signed-off-by: dongjiuzhu1 --- drivers/sensors/sensor.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/sensors/sensor.c b/drivers/sensors/sensor.c index 7717b9af3caa0..7951f840e207b 100644 --- a/drivers/sensors/sensor.c +++ b/drivers/sensors/sensor.c @@ -952,6 +952,7 @@ static int sensor_ioctl(FAR struct file *filep, int cmd, unsigned long arg) user->event = 0; nxrmutex_unlock(&upper->lock); } + break; case SNIOC_FLUSH: { From cd060795408e63c559efc8997b414732aa913d1b Mon Sep 17 00:00:00 2001 From: yintao Date: Fri, 12 Jan 2024 16:19:33 +0800 Subject: [PATCH 04/10] sensors/sensor_rpmsg.c: fix list_delete node is NULL if remote not set CONFIG_SENSOR_RPMSG, local can't receive NS_ACK so sensor_rpmsg_device_ns_bound won't be called, not add sre->node when local stop remote, rpmsg_deinit_vdev will call sensor_rpmsg_ns_unbind_cb if (ept && ept->ns_unbind_cb) ept->ns_unbind_cb(ept); list_delete node is NULL at sensor_rpmsg_ns_unbind_cb Signed-off-by: yintao --- drivers/sensors/sensor_rpmsg.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/sensors/sensor_rpmsg.c b/drivers/sensors/sensor_rpmsg.c index 2fb359b3d56e0..b67c21131859b 100644 --- a/drivers/sensors/sensor_rpmsg.c +++ b/drivers/sensors/sensor_rpmsg.c @@ -1303,7 +1303,11 @@ static void sensor_rpmsg_ns_unbind_cb(FAR struct rpmsg_endpoint *ept) nxrmutex_unlock(&g_dev_lock); nxrmutex_lock(&g_ept_lock); - list_delete(&sre->node); + if (list_in_list(&sre->node)) + { + list_delete(&sre->node); + } + nxrmutex_unlock(&g_ept_lock); nxrmutex_destroy(&sre->lock); From 0eafc790ff781389b7127e49ce9b42dd25bb63c6 Mon Sep 17 00:00:00 2001 From: dongjiuzhu1 Date: Sat, 1 Jun 2024 14:55:38 +0800 Subject: [PATCH 05/10] drivers/sensors: update nbuffer after initialization Signed-off-by: dongjiuzhu1 --- drivers/sensors/sensor_rpmsg.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/sensors/sensor_rpmsg.c b/drivers/sensors/sensor_rpmsg.c index b67c21131859b..9cd7af60d9572 100644 --- a/drivers/sensors/sensor_rpmsg.c +++ b/drivers/sensors/sensor_rpmsg.c @@ -1151,7 +1151,13 @@ static int sensor_rpmsg_publish_handler(FAR struct rpmsg_endpoint *ept, ret = file_open(&file, dev->path, SENSOR_REMOTE | O_CLOEXEC); if (ret >= 0) { - file_ioctl(&file, SNIOC_SET_BUFFER_NUMBER, cell->nbuffer); + ret = file_ioctl(&file, SNIOC_SET_BUFFER_NUMBER, + cell->nbuffer); + if (ret >= 0) + { + dev->lower.nbuffer = cell->nbuffer; + } + file_close(&file); } } From 0a76d4d4f6d26767992df2f75bebeaa2fdc07b8d Mon Sep 17 00:00:00 2001 From: dongjiuzhu1 Date: Sat, 24 Aug 2024 21:06:49 +0800 Subject: [PATCH 06/10] drivers/sensors: avoid using sre lock between rptun thread and hpwrok When the system cannot obtain buffers for a long time, hpwork will hold a lot of buffers. However, due to a deadlock between the rptun thread and hpwork, hpwork cannot send the buffers in time, causing the system to crash. so, avoid hold sre lock in rptun thread. Signed-off-by: dongjiuzhu1 --- drivers/sensors/sensor_rpmsg.c | 42 ++++++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/drivers/sensors/sensor_rpmsg.c b/drivers/sensors/sensor_rpmsg.c index 9cd7af60d9572..157c8464ce0af 100644 --- a/drivers/sensors/sensor_rpmsg.c +++ b/drivers/sensors/sensor_rpmsg.c @@ -251,9 +251,6 @@ static int sensor_rpmsg_ioctl_handler(FAR struct rpmsg_endpoint *ept, static int sensor_rpmsg_ioctlack_handler(FAR struct rpmsg_endpoint *ept, FAR void *data, size_t len, uint32_t src, FAR void *priv); -static void sensor_rpmsg_push_event_one(FAR struct sensor_rpmsg_dev_s *dev, - FAR struct sensor_rpmsg_stub_s *stub, - bool flushed); /**************************************************************************** * Private Data @@ -515,6 +512,43 @@ sensor_rpmsg_alloc_proxy(FAR struct sensor_rpmsg_dev_s *dev, return proxy; } +static +void sensor_rpmsg_push_event_persist(FAR struct sensor_rpmsg_dev_s *dev, + FAR struct sensor_rpmsg_stub_s *stub) +{ + FAR struct sensor_rpmsg_cell_s *cell; + FAR struct sensor_rpmsg_data_s *msg; + FAR struct sensor_rpmsg_ept_s *sre; + uint32_t space; + int ret; + + sre = container_of(stub->ept, struct sensor_rpmsg_ept_s, ept); + msg = rpmsg_get_tx_payload_buffer(&sre->ept, &space, true); + if (!msg) + { + snerr("ERROR: push event persist get buffer failed:%s\n", + rpmsg_get_cpuname(sre->ept.rdev)); + return; + } + + msg->command = SENSOR_RPMSG_PUBLISH; + cell = (FAR struct sensor_rpmsg_cell_s *)(msg + 1); + ret = file_read(&stub->file, cell->data, space - sizeof(*msg) - + sizeof(*cell)); + if (ret > 0) + { + cell->len = ret; + cell->cookie = stub->cookie; + cell->nbuffer = dev->lower.nbuffer; + rpmsg_send_nocopy(&sre->ept, msg, sizeof(*msg) + + ((sizeof(*cell) + ret + 0x7) & ~0x7)); + } + else + { + rpmsg_release_tx_buffer(&sre->ept, msg); + } +} + static FAR struct sensor_rpmsg_stub_s * sensor_rpmsg_alloc_stub(FAR struct sensor_rpmsg_dev_s *dev, FAR struct rpmsg_endpoint *ept, @@ -559,7 +593,7 @@ sensor_rpmsg_alloc_stub(FAR struct sensor_rpmsg_dev_s *dev, if (dev->lower.persist) { - sensor_rpmsg_push_event_one(dev, stub, false); + sensor_rpmsg_push_event_persist(dev, stub); } sensor_rpmsg_unlock(dev); From 690ec8a860128ba3c9fd6d89715eccc5af7ce320 Mon Sep 17 00:00:00 2001 From: dongjiuzhu1 Date: Wed, 10 Jul 2024 17:52:19 +0800 Subject: [PATCH 07/10] drivers/sensors: clear pollpri events after get events Signed-off-by: dongjiuzhu1 --- drivers/sensors/sensor.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/sensors/sensor.c b/drivers/sensors/sensor.c index 7951f840e207b..7ec0530cca204 100644 --- a/drivers/sensors/sensor.c +++ b/drivers/sensors/sensor.c @@ -950,6 +950,7 @@ static int sensor_ioctl(FAR struct file *filep, int cmd, unsigned long arg) nxrmutex_lock(&upper->lock); *(FAR unsigned int *)(uintptr_t)arg = user->event; user->event = 0; + user->changed = false; nxrmutex_unlock(&upper->lock); } break; From d182bd0a8765781728f686039b4963d72242b3a3 Mon Sep 17 00:00:00 2001 From: dongjiuzhu1 Date: Thu, 4 Jul 2024 11:21:44 +0800 Subject: [PATCH 08/10] driver/sensors: add flags O_DIRECT to watch sensor state to avoid rpmsg work Signed-off-by: dongjiuzhu1 --- drivers/sensors/sensor.c | 52 ++++++++++++++++++++++------------------ 1 file changed, 29 insertions(+), 23 deletions(-) diff --git a/drivers/sensors/sensor.c b/drivers/sensors/sensor.c index 7ec0530cca204..389085dd11cfb 100644 --- a/drivers/sensors/sensor.c +++ b/drivers/sensors/sensor.c @@ -615,28 +615,31 @@ static int sensor_open(FAR struct file *filep) } } - if (filep->f_oflags & O_RDOK) + if ((filep->f_oflags & O_DIRECT) == 0) { - if (upper->state.nsubscribers == 0 && lower->ops->activate) + if (filep->f_oflags & O_RDOK) { - ret = lower->ops->activate(lower, filep, true); - if (ret < 0) + if (upper->state.nsubscribers == 0 && lower->ops->activate) { - goto errout_with_open; + ret = lower->ops->activate(lower, filep, true); + if (ret < 0) + { + goto errout_with_open; + } } - } - user->role |= SENSOR_ROLE_RD; - upper->state.nsubscribers++; - } + user->role |= SENSOR_ROLE_RD; + upper->state.nsubscribers++; + } - if (filep->f_oflags & O_WROK) - { - user->role |= SENSOR_ROLE_WR; - upper->state.nadvertisers++; - if (filep->f_oflags & SENSOR_PERSIST) + if (filep->f_oflags & O_WROK) { - lower->persist = true; + user->role |= SENSOR_ROLE_WR; + upper->state.nadvertisers++; + if (filep->f_oflags & SENSOR_PERSIST) + { + lower->persist = true; + } } } @@ -695,18 +698,21 @@ static int sensor_close(FAR struct file *filep) } } - if (filep->f_oflags & O_RDOK) + if ((filep->f_oflags & O_DIRECT) == 0) { - upper->state.nsubscribers--; - if (upper->state.nsubscribers == 0 && lower->ops->activate) + if (filep->f_oflags & O_RDOK) { - lower->ops->activate(lower, filep, false); + upper->state.nsubscribers--; + if (upper->state.nsubscribers == 0 && lower->ops->activate) + { + lower->ops->activate(lower, filep, false); + } } - } - if (filep->f_oflags & O_WROK) - { - upper->state.nadvertisers--; + if (filep->f_oflags & O_WROK) + { + upper->state.nadvertisers--; + } } list_delete(&user->node); From 4a25277cbb871b03f671a926e308f05b1b591a68 Mon Sep 17 00:00:00 2001 From: dongjiuzhu1 Date: Mon, 22 Jul 2024 12:59:40 +0800 Subject: [PATCH 09/10] drivers/sensors: Fixed the overflow problem of uint32_t subtracting large from small. Signed-off-by: likun17 --- drivers/sensors/sensor.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/sensors/sensor.c b/drivers/sensors/sensor.c index 389085dd11cfb..6fa359e0d6381 100644 --- a/drivers/sensors/sensor.c +++ b/drivers/sensors/sensor.c @@ -399,7 +399,7 @@ static void sensor_generate_timing(FAR struct sensor_upperhalf_s *upper, static bool sensor_is_updated(FAR struct sensor_upperhalf_s *upper, FAR struct sensor_user_s *user) { - long delta = upper->state.generation - user->state.generation; + long delta = (long long)upper->state.generation - user->state.generation; if (delta <= 0) { @@ -431,7 +431,7 @@ static void sensor_catch_up(FAR struct sensor_upperhalf_s *upper, long delta; circbuf_peek(&upper->timing, &generation, TIMING_BUF_ESIZE); - delta = generation - user->state.generation; + delta = (long long)generation - user->state.generation; if (delta > 0) { user->bufferpos = upper->timing.tail / TIMING_BUF_ESIZE; From cfdcb4e32324b5192a542d4342864524e206e611 Mon Sep 17 00:00:00 2001 From: dongjiuzhu1 Date: Mon, 27 May 2024 21:12:34 +0800 Subject: [PATCH 10/10] drivers/sensor: remove frequent logs Signed-off-by: dongjiuzhu1 --- drivers/sensors/sensor.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/sensors/sensor.c b/drivers/sensors/sensor.c index 6fa359e0d6381..9465e52727922 100644 --- a/drivers/sensors/sensor.c +++ b/drivers/sensors/sensor.c @@ -822,8 +822,6 @@ static int sensor_ioctl(FAR struct file *filep, int cmd, unsigned long arg) uint32_t arg1 = (uint32_t)arg; int ret = 0; - sninfo("cmd=%x arg=%08lx\n", cmd, arg); - switch (cmd) { case SNIOC_GET_STATE: