mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Maoyi Xie <maoyixie.tju@gmail.com>
To: "Alex Deucher" <alexander.deucher@amd.com>,
	"Christian König" <christian.koenig@amd.com>
Cc: David Airlie <airlied@gmail.com>, Simona Vetter <simona@ffwll.ch>,
	amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	linux-kernel@vger.kernel.org
Subject: drm/amdgpu: dead empty checks on e->list in ring_mux ib_mark_offset and end_ib?
Date: Wed, 20 May 2026 03:41:13 +0800	[thread overview]
Message-ID: <20260519194113.2411822-1-maoyixie.tju@gmail.com> (raw)

Hi all,

While auditing list_last_entry callsites, I noticed two places in
drivers/gpu/drm/amd/amdgpu/amdgpu_ring_mux.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 these are worth fixing.

The two sites are amdgpu_ring_mux_ib_mark_offset() and
amdgpu_ring_mux_end_ib() (linux-7.1-rc1, around lines 497 and
530):

    chunk = list_last_entry(&e->list, struct amdgpu_mux_chunk, entry);
    if (!chunk) {
            DRM_ERROR("cannot find chunk!\n");
            return;
    }

list_last_entry() returns container_of(&e->list, struct
amdgpu_mux_chunk, entry) when e->list is empty, never NULL. The
"cannot find chunk!" error path is dead code.

With an empty e->list, the fall through pointer aliases &e->list
inside struct amdgpu_mux_entry. The writes that follow then
corrupt fields of the mux_entry at the corresponding offsets.
mark_offset writes cntl_offset, de_offset and ce_offset. end_ib
writes end and sync_seq.

e->list is empty if a software ring submits an IB mark or IB end
before any chunk is queued for that ring. This can happen on a
fresh start_ib path, or after end_ib drops the last chunk.

A candidate fix is a one liner per site. Switch to
list_last_entry_or_null so the existing error path runs.

Similar dead empty checks after list_first_entry / list_last_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. These two sites appear 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/

             reply	other threads:[~2026-05-19 19:41 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-19 19:41 Maoyi Xie [this message]
2026-05-20  8:22 ` Christian König

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=20260519194113.2411822-1-maoyixie.tju@gmail.com \
    --to=maoyixie.tju@gmail.com \
    --cc=airlied@gmail.com \
    --cc=alexander.deucher@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=christian.koenig@amd.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=simona@ffwll.ch \
    /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®