From aa567d43f6e341e2df0bf48817d841158f2dbb71 Mon Sep 17 00:00:00 2001 From: Yongrong Wang Date: Sat, 8 Feb 2025 10:38:16 +0800 Subject: [PATCH] drivers/rpmsg: fix struct alignment issue by removing zero array member This commit fixes a memory alignment issue in the rpmsg subsystem. The previous implementation used a flexible array member (rdev[0]) in struct rpmsg_s, but due to automatic 8-byte padding, rpmsg->rdev could differ from the transport layer's rdev address (e.g., rpmsg_virtio->rvdev.rdev), causing potential memory corruption. Changes: - Remove rdev[0] flexible array member from struct rpmsg_s - Add rpmsg_get_rdev_by_rpmsg() inline helper to calculate rdev address as (rpmsg + 1), ensuring correct pointer arithmetic - Update all rpmsg->rdev references to use the new helper function - Fix rpmsg_virtio_notify_wait() to use correct container_of macro Signed-off-by: Yongrong Wang --- drivers/rpmsg/rpmsg.c | 30 +++++++++++++++++------------- drivers/rpmsg/rpmsg_port.c | 5 +++-- drivers/rpmsg/rpmsg_virtio.c | 6 +++--- drivers/rpmsg/rpmsg_virtio_lite.c | 2 +- include/nuttx/rpmsg/rpmsg.h | 12 +++++++++++- 5 files changed, 35 insertions(+), 20 deletions(-) diff --git a/drivers/rpmsg/rpmsg.c b/drivers/rpmsg/rpmsg.c index 681c1359a8f..ae2f786019e 100644 --- a/drivers/rpmsg/rpmsg.c +++ b/drivers/rpmsg/rpmsg.c @@ -96,7 +96,7 @@ rpmsg_get_by_rdev(FAR struct rpmsg_device *rdev) return NULL; } - return metal_container_of(rdev, struct rpmsg_s, rdev); + return (FAR struct rpmsg_s *)((FAR char *)rdev - sizeof(struct rpmsg_s)); } static int rpmsg_dev_ioctl_(FAR struct rpmsg_s *rpmsg, int cmd, @@ -248,6 +248,7 @@ int rpmsg_register_callback(FAR void *priv, { FAR struct rpmsg_s *rpmsg = metal_container_of(node, struct rpmsg_s, node); + FAR struct rpmsg_device *rdev = rpmsg_get_rdev_by_rpmsg(rpmsg); if (!rpmsg->init) { @@ -256,7 +257,7 @@ int rpmsg_register_callback(FAR void *priv, if (device_created) { - device_created(rpmsg->rdev, priv); + device_created(rdev, priv); } if (ns_bind == NULL) @@ -273,11 +274,11 @@ again: FAR struct rpmsg_bind_s *bind = metal_container_of(bnode, struct rpmsg_bind_s, node); - if (ns_match(rpmsg->rdev, priv, bind->name, bind->dest)) + if (ns_match(rdev, priv, bind->name, bind->dest)) { metal_list_del(bnode); nxrmutex_unlock(&rpmsg->lock); - ns_bind(rpmsg->rdev, priv, bind->name, bind->dest); + ns_bind(rdev, priv, bind->name, bind->dest); kmm_free(bind); goto again; @@ -326,10 +327,11 @@ void rpmsg_unregister_callback(FAR void *priv, { FAR struct rpmsg_s *rpmsg = metal_container_of(pnode, struct rpmsg_s, node); + FAR struct rpmsg_device *rdev = rpmsg_get_rdev_by_rpmsg(rpmsg); if (rpmsg->init) { - device_destroy(rpmsg->rdev, priv); + device_destroy(rdev, priv); } } } @@ -403,6 +405,7 @@ void rpmsg_ns_unbind(FAR struct rpmsg_device *rdev, void rpmsg_device_created(FAR struct rpmsg_s *rpmsg) { + FAR struct rpmsg_device *rdev = rpmsg_get_rdev_by_rpmsg(rpmsg); FAR struct metal_list *node; FAR struct metal_list *tmp; @@ -414,7 +417,7 @@ void rpmsg_device_created(FAR struct rpmsg_s *rpmsg) if (cb->device_created) { - cb->device_created(rpmsg->rdev, cb->priv); + cb->device_created(rdev, cb->priv); } } @@ -422,15 +425,16 @@ void rpmsg_device_created(FAR struct rpmsg_s *rpmsg) up_write(&g_rpmsg_lock); #ifdef CONFIG_RPMSG_PING - rpmsg_ping_init(rpmsg->rdev, &rpmsg->ping); + rpmsg_ping_init(rdev, &rpmsg->ping); #endif #ifdef CONFIG_RPMSG_TEST - rpmsg_test_init(rpmsg->rdev, &rpmsg->test); + rpmsg_test_init(rdev, &rpmsg->test); #endif } void rpmsg_device_destory(FAR struct rpmsg_s *rpmsg) { + FAR struct rpmsg_device *rdev = rpmsg_get_rdev_by_rpmsg(rpmsg); FAR struct metal_list *node; FAR struct metal_list *tmp; FAR struct rpmsg_endpoint *ept; @@ -467,7 +471,7 @@ void rpmsg_device_destory(FAR struct rpmsg_s *rpmsg) if (cb->device_destroy) { - cb->device_destroy(rpmsg->rdev, cb->priv); + cb->device_destroy(rdev, cb->priv); } } @@ -475,8 +479,8 @@ void rpmsg_device_destory(FAR struct rpmsg_s *rpmsg) /* Release all ept attached to current rpmsg device */ - metal_mutex_acquire(&rpmsg->rdev->lock); - metal_list_for_each_safe(&rpmsg->rdev->endpoints, tmp, node) + metal_mutex_acquire(&rdev->lock); + metal_list_for_each_safe(&rdev->endpoints, tmp, node) { ept = metal_container_of(node, struct rpmsg_endpoint, node); if (ept->ns_unbind_cb) @@ -489,7 +493,7 @@ void rpmsg_device_destory(FAR struct rpmsg_s *rpmsg) } } - metal_mutex_release(&rpmsg->rdev->lock); + metal_mutex_release(&rdev->lock); } int rpmsg_register(FAR const char *path, FAR struct rpmsg_s *rpmsg, @@ -583,7 +587,7 @@ void rpmsg_dump_all(void) void rpmsg_modify_signals(FAR struct rpmsg_s *rpmsg, int setflags, int clrflags) { - FAR struct rpmsg_device *rdev = rpmsg->rdev; + FAR struct rpmsg_device *rdev = rpmsg_get_rdev_by_rpmsg(rpmsg); FAR struct rpmsg_endpoint *ept; FAR struct metal_list *node; bool needlock; diff --git a/drivers/rpmsg/rpmsg_port.c b/drivers/rpmsg/rpmsg_port.c index bec6771d3ba..79538d8d3c2 100644 --- a/drivers/rpmsg/rpmsg_port.c +++ b/drivers/rpmsg/rpmsg_port.c @@ -594,8 +594,9 @@ int rpmsg_port_initialize(FAR struct rpmsg_port_s *port, void rpmsg_port_uninitialize(FAR struct rpmsg_port_s *port) { FAR struct rpmsg_s *rpmsg = &port->rpmsg; + FAR struct rpmsg_device *rdev = rpmsg_get_rdev_by_rpmsg(rpmsg); - metal_mutex_deinit(&rpmsg->rdev->lock); + metal_mutex_deinit(&rdev->lock); rpmsg_port_destroy_queues(port); } @@ -789,7 +790,7 @@ static void rpmsg_port_dump_buffer(FAR struct rpmsg_device *rdev, static void rpmsg_port_dump(FAR struct rpmsg_s *rpmsg) { FAR struct rpmsg_port_s *port = (FAR struct rpmsg_port_s *)rpmsg; - FAR struct rpmsg_device *rdev = rpmsg->rdev; + FAR struct rpmsg_device *rdev = rpmsg_get_rdev_by_rpmsg(rpmsg); FAR struct rpmsg_endpoint *ept; FAR struct metal_list *node; bool needunlock = false; diff --git a/drivers/rpmsg/rpmsg_virtio.c b/drivers/rpmsg/rpmsg_virtio.c index 20667a9eb89..66a44fe0840 100644 --- a/drivers/rpmsg/rpmsg_virtio.c +++ b/drivers/rpmsg/rpmsg_virtio.c @@ -403,7 +403,7 @@ static void rpmsg_virtio_dump(FAR struct rpmsg_s *rpmsg) FAR struct rpmsg_virtio_priv_s *priv = (FAR struct rpmsg_virtio_priv_s *)rpmsg; FAR struct rpmsg_virtio_device *rvdev = &priv->rvdev; - FAR struct rpmsg_device *rdev = rpmsg->rdev; + FAR struct rpmsg_device *rdev = &rvdev->rdev; FAR struct rpmsg_endpoint *ept; FAR struct metal_list *node; bool needunlock = false; @@ -515,8 +515,8 @@ static void rpmsg_virtio_tx_notify(FAR struct virtqueue *vq) static int rpmsg_virtio_notify_wait(FAR struct rpmsg_device *rdev, uint32_t id) { - FAR struct rpmsg_virtio_priv_s *priv = (FAR struct rpmsg_virtio_priv_s *) - metal_container_of(rdev, struct rpmsg_s, rdev); + FAR struct rpmsg_virtio_priv_s *priv = + metal_container_of(rdev, struct rpmsg_virtio_priv_s, rvdev); if (!rpmsg_virtio_is_recursive(priv)) { diff --git a/drivers/rpmsg/rpmsg_virtio_lite.c b/drivers/rpmsg/rpmsg_virtio_lite.c index 2920ff665e9..65bf52a59e6 100644 --- a/drivers/rpmsg/rpmsg_virtio_lite.c +++ b/drivers/rpmsg/rpmsg_virtio_lite.c @@ -475,8 +475,8 @@ static void rpmsg_virtio_lite_dump(FAR struct rpmsg_s *rpmsg) { FAR struct rpmsg_virtio_lite_priv_s *priv = (FAR struct rpmsg_virtio_lite_priv_s *)rpmsg; + FAR struct rpmsg_device *rdev = rpmsg_get_rdev_by_rpmsg(rpmsg); FAR struct rpmsg_virtio_device *rvdev = &priv->rvdev; - FAR struct rpmsg_device *rdev = rpmsg->rdev; FAR struct rpmsg_endpoint *ept; FAR struct metal_list *node; bool needlock = true; diff --git a/include/nuttx/rpmsg/rpmsg.h b/include/nuttx/rpmsg/rpmsg.h index db4594cd814..86d11f195b0 100644 --- a/include/nuttx/rpmsg/rpmsg.h +++ b/include/nuttx/rpmsg/rpmsg.h @@ -68,7 +68,6 @@ struct rpmsg_s struct rpmsg_endpoint test; #endif atomic_int signals; - struct rpmsg_device rdev[0]; }; /** @@ -108,6 +107,17 @@ extern "C" #define EXTERN extern #endif +static inline FAR struct rpmsg_device * +rpmsg_get_rdev_by_rpmsg(FAR struct rpmsg_s *rpmsg) +{ + if (!rpmsg) + { + return NULL; + } + + return (FAR struct rpmsg_device *)(rpmsg + 1); +} + int rpmsg_wait(FAR struct rpmsg_endpoint *ept, FAR sem_t *sem); int rpmsg_post(FAR struct rpmsg_endpoint *ept, FAR sem_t *sem);