mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* RDMA/hns: dead empty check on head->root in setup_root_hem?
@ 2026-05-19 20:28 Maoyi Xie
  2026-05-20  6:04 ` Junxian Huang
  0 siblings, 1 reply; 2+ messages in thread
From: Maoyi Xie @ 2026-05-19 20:28 UTC (permalink / raw)
  To: Chengchang Tang, Junxian Huang
  Cc: Jason Gunthorpe, Leon Romanovsky, linux-rdma, linux-kernel

Hi all,

While auditing list_first_entry callsites, I noticed a place in
drivers/infiniband/hw/hns/hns_roce_hem.c where the developer
wrote a NULL check for an empty list but used the unsafe API.
The check is dead code. I would appreciate it if you could take
a look and let me know whether this is worth fixing.

The site is setup_root_hem() (linux-7.1-rc1, around line 1270):

    root_hem = list_first_entry(&head->root,
                                struct hns_roce_hem_item, list);
    if (!root_hem)
            return -ENOMEM;

    total = 0;
    for (i = 0; i < region_cnt && total <= max_ba_num; i++) {
            ...
            cpu_base = root_hem->addr + total * BA_BYTE_LEN;
            ...

list_first_entry() returns container_of(&head->root, struct
hns_roce_hem_item, list) when head->root is empty, never NULL.
The -ENOMEM early return is dead code.

With an empty head->root, the fall through pointer aliases
&head->root inside struct hns_roce_hem_head. root_hem->addr is
then read from memory at a fixed offset inside that struct,
producing a garbage DMA base address that is then handed to
the hardware.

head->root is empty if the caller fails to allocate and add the
root item before calling setup_root_hem. A future caller could
also miss that precondition.

A candidate fix is a one liner. Switch to list_first_entry_or_null
so the -ENOMEM early return runs.

Similar dead empty checks after list_first_entry have been
cleaned up in the same shape, for example commit fbb8bc408027
(net: qed: Remove redundant NULL checks after list_first_entry),
commit c708d3fad421 (crypto: atmel: use list_first_entry_or_null
to simplify find_dev) and commit 10379171f346 (ksmbd: use
list_first_entry_or_null for opinfo_get_list). The qed commit
message describes the exact shape we observe here. This hns
site appears to be missed by those cleanups.

If this is intentional or already known, please disregard.
Otherwise I am happy to send a [PATCH] or to leave the fix to
you.

Thanks,
Maoyi Xie
https://maoyixie.com/

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: RDMA/hns: dead empty check on head->root in setup_root_hem?
  2026-05-19 20:28 RDMA/hns: dead empty check on head->root in setup_root_hem? Maoyi Xie
@ 2026-05-20  6:04 ` Junxian Huang
  0 siblings, 0 replies; 2+ messages in thread
From: Junxian Huang @ 2026-05-20  6:04 UTC (permalink / raw)
  To: Maoyi Xie, Chengchang Tang
  Cc: Jason Gunthorpe, Leon Romanovsky, linux-rdma, linux-kernel



On 2026/5/20 4:28, Maoyi Xie wrote:
> Hi all,
> 
> While auditing list_first_entry callsites, I noticed a place in
> drivers/infiniband/hw/hns/hns_roce_hem.c where the developer
> wrote a NULL check for an empty list but used the unsafe API.
> The check is dead code. I would appreciate it if you could take
> a look and let me know whether this is worth fixing.
> 
> The site is setup_root_hem() (linux-7.1-rc1, around line 1270):
> 
>     root_hem = list_first_entry(&head->root,
>                                 struct hns_roce_hem_item, list);
>     if (!root_hem)
>             return -ENOMEM;
> 
>     total = 0;
>     for (i = 0; i < region_cnt && total <= max_ba_num; i++) {
>             ...
>             cpu_base = root_hem->addr + total * BA_BYTE_LEN;
>             ...
> 
> list_first_entry() returns container_of(&head->root, struct
> hns_roce_hem_item, list) when head->root is empty, never NULL.
> The -ENOMEM early return is dead code.
> 
> With an empty head->root, the fall through pointer aliases
> &head->root inside struct hns_roce_hem_head. root_hem->addr is
> then read from memory at a fixed offset inside that struct,
> producing a garbage DMA base address that is then handed to
> the hardware.
> 
> head->root is empty if the caller fails to allocate and add the
> root item before calling setup_root_hem. A future caller could
> also miss that precondition.
> 
> A candidate fix is a one liner. Switch to list_first_entry_or_null
> so the -ENOMEM early return runs.
> 
> Similar dead empty checks after list_first_entry have been
> cleaned up in the same shape, for example commit fbb8bc408027
> (net: qed: Remove redundant NULL checks after list_first_entry),
> commit c708d3fad421 (crypto: atmel: use list_first_entry_or_null
> to simplify find_dev) and commit 10379171f346 (ksmbd: use
> list_first_entry_or_null for opinfo_get_list). The qed commit
> message describes the exact shape we observe here. This hns
> site appears to be missed by those cleanups.
> 
> If this is intentional or already known, please disregard.
> Otherwise I am happy to send a [PATCH] or to leave the fix to
> you.

Please send a patch, thanks!

Junxian

> 
> Thanks,
> Maoyi Xie
> https://maoyixie.com/

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-05-20  6:04 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-05-19 20:28 RDMA/hns: dead empty check on head->root in setup_root_hem? Maoyi Xie
2026-05-20  6:04 ` Junxian Huang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®