From 39e210a148c95f0e74b91ae0645ba8ffa9e3ca7a Mon Sep 17 00:00:00 2001 From: Aurora zuma automerger Date: Fri, 30 Jun 2023 07:01:47 +0000 Subject: gxp: [Copybara Auto Merge] Merge branch 'zuma' into 'android14-gs-pixel-5.15-udc-d1' gxp: introduce locked mapping search and remove Bug: 289468717 Test: Passed the unittests; Can't reproduce the bug with Syzkaller (cherry picked from commit b66c271952702040b61ceeb420d626e7c2651421) gcip: Always use irqsave for ACQUIRE_WAIT_LIST_LOCK In order to prevent deadlock causing by interrupt, always use irqsave when acquiring wait list lock. Bug: 288023845 Test: sh run.sh --chip rio --syz-reproduce (cherry picked from commit 8da104b0e05fe339a5b8da3e5176fd84d8a61135) GitOrigin-RevId: e9bfe341b20a0954393f381228ac7e733222c202 Change-Id: I98edc63451050f4de7afea6800362bb35a818920 --- gcip-kernel-driver/drivers/gcip/gcip-mailbox.c | 5 ++-- gxp-common-platform.c | 40 +++++++++++++++----------- gxp-vd.c | 34 +++++++++++++++++----- gxp-vd.h | 13 +++++++++ 4 files changed, 66 insertions(+), 26 deletions(-) diff --git a/gcip-kernel-driver/drivers/gcip/gcip-mailbox.c b/gcip-kernel-driver/drivers/gcip/gcip-mailbox.c index afa67c8..cdc76b0 100644 --- a/gcip-kernel-driver/drivers/gcip/gcip-mailbox.c +++ b/gcip-kernel-driver/drivers/gcip/gcip-mailbox.c @@ -457,6 +457,7 @@ static void gcip_mailbox_flush_awaiter(struct gcip_mailbox *mailbox) struct gcip_mailbox_wait_list_elem *cur, *nxt; struct gcip_mailbox_resp_awaiter *awaiter; struct list_head resps_to_flush; + unsigned long flags; /* If mailbox->ops is NULL, the mailbox is already released. */ if (!mailbox->ops) @@ -469,7 +470,7 @@ static void gcip_mailbox_flush_awaiter(struct gcip_mailbox *mailbox) * handled already. */ INIT_LIST_HEAD(&resps_to_flush); - ACQUIRE_WAIT_LIST_LOCK(false, NULL); + ACQUIRE_WAIT_LIST_LOCK(true, &flags); list_for_each_entry_safe (cur, nxt, &mailbox->wait_list, list) { list_del(&cur->list); if (cur->awaiter) { @@ -490,7 +491,7 @@ static void gcip_mailbox_flush_awaiter(struct gcip_mailbox *mailbox) kfree(cur); } } - RELEASE_WAIT_LIST_LOCK(false, 0); + RELEASE_WAIT_LIST_LOCK(true, flags); /* * Cancel the timeout timer of and free any responses that were still in diff --git a/gxp-common-platform.c b/gxp-common-platform.c index 13bfbc2..5903d07 100644 --- a/gxp-common-platform.c +++ b/gxp-common-platform.c @@ -277,8 +277,7 @@ error_destroy: return ret; } -static int gxp_unmap_buffer(struct gxp_client *client, - struct gxp_map_ioctl __user *argp) +static int gxp_unmap_buffer(struct gxp_client *client, struct gxp_map_ioctl __user *argp) { struct gxp_dev *gxp = client->gxp; struct gxp_map_ioctl ibuf; @@ -297,17 +296,20 @@ static int gxp_unmap_buffer(struct gxp_client *client, goto out; } - map = gxp_vd_mapping_search(client->vd, - (dma_addr_t)ibuf.device_address); + down_write(&client->vd->mappings_semaphore); + + map = gxp_vd_mapping_search_locked(client->vd, (dma_addr_t)ibuf.device_address); if (!map) { - dev_err(gxp->dev, - "Mapping not found for provided device address %#llX\n", + dev_err(gxp->dev, "Mapping not found for provided device address %#llX\n", ibuf.device_address); ret = -EINVAL; - goto out; } else if (!map->host_address) { dev_err(gxp->dev, "dma-bufs must be unmapped via GXP_UNMAP_DMABUF\n"); ret = -EINVAL; + } + + if (ret) { + up_write(&client->vd->mappings_semaphore); goto out_put; } @@ -316,13 +318,15 @@ static int gxp_unmap_buffer(struct gxp_client *client, gxp->dev, "The host address of the unmap request is different from the original one\n"); - gxp_vd_mapping_remove(client->vd, map); - gxp_mapping_iova_log(client, map, - GXP_IOVA_LOG_UNMAP | GXP_IOVA_LOG_BUFFER); + gxp_vd_mapping_remove_locked(client->vd, map); + up_write(&client->vd->mappings_semaphore); + + gxp_mapping_iova_log(client, map, GXP_IOVA_LOG_UNMAP | GXP_IOVA_LOG_BUFFER); out_put: /* Release the reference from gxp_vd_mapping_search() */ - gxp_mapping_put(map); + if (map) + gxp_mapping_put(map); out: up_read(&client->semaphore); @@ -1294,8 +1298,7 @@ out_unlock: return ret; } -static int gxp_unmap_dmabuf(struct gxp_client *client, - struct gxp_map_dmabuf_ioctl __user *argp) +static int gxp_unmap_dmabuf(struct gxp_client *client, struct gxp_map_dmabuf_ioctl __user *argp) { struct gxp_dev *gxp = client->gxp; struct gxp_map_dmabuf_ioctl ibuf; @@ -1314,14 +1317,17 @@ static int gxp_unmap_dmabuf(struct gxp_client *client, goto out; } + down_write(&client->vd->mappings_semaphore); + /* * Fetch and remove the internal mapping records. * If host_address is not 0, the provided device_address belongs to a * non-dma-buf mapping. */ - mapping = gxp_vd_mapping_search(client->vd, ibuf.device_address); + mapping = gxp_vd_mapping_search_locked(client->vd, ibuf.device_address); if (IS_ERR_OR_NULL(mapping) || mapping->host_address) { dev_warn(gxp->dev, "No dma-buf mapped for given IOVA\n"); + up_write(&client->vd->mappings_semaphore); /* * If the device address belongs to a non-dma-buf mapping, * release the reference to it obtained via the search. @@ -1333,10 +1339,10 @@ static int gxp_unmap_dmabuf(struct gxp_client *client, } /* Remove the mapping from its VD, releasing the VD's reference */ - gxp_vd_mapping_remove(client->vd, mapping); + gxp_vd_mapping_remove_locked(client->vd, mapping); + up_write(&client->vd->mappings_semaphore); - gxp_mapping_iova_log(client, mapping, - GXP_IOVA_LOG_UNMAP | GXP_IOVA_LOG_DMABUF); + gxp_mapping_iova_log(client, mapping, GXP_IOVA_LOG_UNMAP | GXP_IOVA_LOG_DMABUF); /* Release the reference from gxp_vd_mapping_search() */ gxp_mapping_put(mapping); diff --git a/gxp-vd.c b/gxp-vd.c index 7755d56..5c0fb86 100644 --- a/gxp-vd.c +++ b/gxp-vd.c @@ -1272,14 +1272,19 @@ void gxp_vd_mapping_remove(struct gxp_virtual_device *vd, struct gxp_mapping *map) { down_write(&vd->mappings_semaphore); + gxp_vd_mapping_remove_locked(vd, map); + up_write(&vd->mappings_semaphore); +} + +void gxp_vd_mapping_remove_locked(struct gxp_virtual_device *vd, struct gxp_mapping *map) +{ + lockdep_assert_held_write(&vd->mappings_semaphore); /* Drop the mapping from this virtual device's records */ rb_erase(&map->node, &vd->mappings_root); /* Release the reference obtained in gxp_vd_mapping_store() */ gxp_mapping_put(map); - - up_write(&vd->mappings_semaphore); } static bool is_device_address_in_mapping(struct gxp_mapping *mapping, @@ -1296,7 +1301,7 @@ gxp_vd_mapping_internal_search(struct gxp_virtual_device *vd, struct rb_node *node; struct gxp_mapping *mapping; - down_read(&vd->mappings_semaphore); + lockdep_assert_held(&vd->mappings_semaphore); node = vd->mappings_root.rb_node; @@ -1306,7 +1311,6 @@ gxp_vd_mapping_internal_search(struct gxp_virtual_device *vd, (check_range && is_device_address_in_mapping(mapping, device_address))) { gxp_mapping_get(mapping); - up_read(&vd->mappings_semaphore); return mapping; /* Found it */ } else if (mapping->device_address > device_address) { node = node->rb_left; @@ -1315,13 +1319,23 @@ gxp_vd_mapping_internal_search(struct gxp_virtual_device *vd, } } - up_read(&vd->mappings_semaphore); - return NULL; } struct gxp_mapping *gxp_vd_mapping_search(struct gxp_virtual_device *vd, dma_addr_t device_address) +{ + struct gxp_mapping *mapping; + + down_read(&vd->mappings_semaphore); + mapping = gxp_vd_mapping_search_locked(vd, device_address); + up_read(&vd->mappings_semaphore); + + return mapping; +} + +struct gxp_mapping *gxp_vd_mapping_search_locked(struct gxp_virtual_device *vd, + dma_addr_t device_address) { return gxp_vd_mapping_internal_search(vd, device_address, false); } @@ -1330,7 +1344,13 @@ struct gxp_mapping * gxp_vd_mapping_search_in_range(struct gxp_virtual_device *vd, dma_addr_t device_address) { - return gxp_vd_mapping_internal_search(vd, device_address, true); + struct gxp_mapping *mapping; + + down_read(&vd->mappings_semaphore); + mapping = gxp_vd_mapping_internal_search(vd, device_address, true); + up_read(&vd->mappings_semaphore); + + return mapping; } struct gxp_mapping *gxp_vd_mapping_search_host(struct gxp_virtual_device *vd, diff --git a/gxp-vd.h b/gxp-vd.h index f9f9bad..d33e518 100644 --- a/gxp-vd.h +++ b/gxp-vd.h @@ -274,6 +274,12 @@ int gxp_vd_mapping_store(struct gxp_virtual_device *vd, void gxp_vd_mapping_remove(struct gxp_virtual_device *vd, struct gxp_mapping *map); +/** + * gxp_vd_mapping_remove_locked() - The same as `gxp_vd_mapping_remove` but the caller holds + * @vd->mappings_semaphore as write. + */ +void gxp_vd_mapping_remove_locked(struct gxp_virtual_device *vd, struct gxp_mapping *map); + /** * gxp_vd_mapping_search() - Obtain a reference to the mapping starting at the * specified device address @@ -287,6 +293,13 @@ void gxp_vd_mapping_remove(struct gxp_virtual_device *vd, struct gxp_mapping *gxp_vd_mapping_search(struct gxp_virtual_device *vd, dma_addr_t device_address); +/** + * gxp_vd_mapping_search_locked() - The same as `gxp_vd_mapping_search` but the caller holds + * @vd->mappings_semaphore. + */ +struct gxp_mapping *gxp_vd_mapping_search_locked(struct gxp_virtual_device *vd, + dma_addr_t device_address); + /** * gxp_vd_mapping_search_in_range() - Obtain a reference to the mapping which * contains the specified device address -- cgit v1.2.3