From: Junxian Huang <huangjunxian6@hisilicon.com>
To: Maoyi Xie <maoyixie.tju@gmail.com>,
Chengchang Tang <tangchengchang@huawei.com>
Cc: Jason Gunthorpe <jgg@ziepe.ca>, Leon Romanovsky <leon@kernel.org>,
<linux-rdma@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: RDMA/hns: dead empty check on head->root in setup_root_hem?
Date: Wed, 20 May 2026 14:04:42 +0800 [thread overview]
Message-ID: <8fca4982-3046-95d1-27c7-0766105db73e@hisilicon.com> (raw)
In-Reply-To: <20260519202809.2437430-1-maoyixie.tju@gmail.com>
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/
prev parent reply other threads:[~2026-05-20 6:04 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-19 20:28 Maoyi Xie
2026-05-20 6:04 ` Junxian Huang [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8fca4982-3046-95d1-27c7-0766105db73e@hisilicon.com \
--to=huangjunxian6@hisilicon.com \
--cc=jgg@ziepe.ca \
--cc=leon@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rdma@vger.kernel.org \
--cc=maoyixie.tju@gmail.com \
--cc=tangchengchang@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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®