linux-kernel - Re: [PATCH] driver: soc: xilinx: use _safe loop iterator to avoid a use after free

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <58711e80-8720-0a92-2e9e-5ae0a23368c3@amd.com>
Date:   Fri, 12 May 2023 13:08:55 +0200
From:   Michal Simek <michal.simek@....com>
To:     Dan Carpenter <dan.carpenter@...aro.org>,
        Abhyuday Godhasara <abhyuday.godhasara@...inx.com>
Cc:     Michal Simek <michal.simek@...inx.com>,
        Rajan Vaja <rajan.vaja@...inx.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Tejas Patel <tejas.patel@...inx.com>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        kernel-janitors@...r.kernel.org
Subject: Re: [PATCH] driver: soc: xilinx: use _safe loop iterator to avoid a
 use after free



On 4/21/23 12:44, Dan Carpenter wrote:
> CAUTION: This message has originated from an External Source. Please use proper judgment and caution when opening attachments, clicking links, or responding to this email.
> 
> 
> The hash_for_each_possible() loop dereferences "eve_data" to get the
> next item on the list.  However the loop frees eve_data so it leads to
> a use after free.  Use hash_for_each_possible_safe() instead.
> 
> Fixes: c7fdb2404f66 ("drivers: soc: xilinx: add xilinx event management driver")
> Signed-off-by: Dan Carpenter <dan.carpenter@...aro.org>
> ---
> Found by static analysis and not tested.
> 
>   drivers/soc/xilinx/xlnx_event_manager.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/soc/xilinx/xlnx_event_manager.c b/drivers/soc/xilinx/xlnx_event_manager.c
> index c76381899ef4..f9d9b82b562d 100644
> --- a/drivers/soc/xilinx/xlnx_event_manager.c
> +++ b/drivers/soc/xilinx/xlnx_event_manager.c
> @@ -192,11 +192,12 @@ static int xlnx_remove_cb_for_suspend(event_cb_func_t cb_fun)
>          struct registered_event_data *eve_data;
>          struct agent_cb *cb_pos;
>          struct agent_cb *cb_next;
> +       struct hlist_node *tmp;
> 
>          is_need_to_unregister = false;
> 
>          /* Check for existing entry in hash table for given cb_type */
> -       hash_for_each_possible(reg_driver_map, eve_data, hentry, PM_INIT_SUSPEND_CB) {
> +       hash_for_each_possible_safe(reg_driver_map, eve_data, tmp, hentry, PM_INIT_SUSPEND_CB) {
>                  if (eve_data->cb_type == PM_INIT_SUSPEND_CB) {
>                          /* Delete the list of callback */
>                          list_for_each_entry_safe(cb_pos, cb_next, &eve_data->cb_list_head, list) {
> @@ -228,11 +229,12 @@ static int xlnx_remove_cb_for_notify_event(const u32 node_id, const u32 event,
>          u64 key = ((u64)node_id << 32U) | (u64)event;
>          struct agent_cb *cb_pos;
>          struct agent_cb *cb_next;
> +       struct hlist_node *tmp;
> 
>          is_need_to_unregister = false;
> 
>          /* Check for existing entry in hash table for given key id */
> -       hash_for_each_possible(reg_driver_map, eve_data, hentry, key) {
> +       hash_for_each_possible_safe(reg_driver_map, eve_data, tmp, hentry, key) {
>                  if (eve_data->key == key) {
>                          /* Delete the list of callback */
>                          list_for_each_entry_safe(cb_pos, cb_next, &eve_data->cb_list_head, list) {
> --
> 2.39.2
> 

Applied.
M