* Re: [PATCH v2] gpu: host1x: Fix device reference leak in device_add() error path
From: Guangshuo Li @ 2026-04-20 9:05 UTC (permalink / raw)
To: Mikko Perttunen
Cc: Thierry Reding, David Airlie, Simona Vetter,
Vamsee Vardhan Thummala, linux-kernel, dri-devel, linux-tegra,
stable
In-Reply-To: <q6Ni253ETr-zY8OZRWnm4g@nvidia.com>
Hi Mikko,
Thanks for reviewing.
On Mon, 20 Apr 2026 at 15:19, Mikko Perttunen <mperttunen@nvidia.com> wrote:
>
> Unrelated ..
>
Sorry about the unrelated change in drivers/firmware/edd.c. It was
included by mistake due to my carelessness when doing git add.
> This isn't a leak -- if device_add fails, the device is still on the
> device list, though in a "stuck" state, and will get cleaned up through
> host1x_device_del.
>
You're right. I misunderstood this path: if device_add() fails here,
the device remains on host1x->devices and can still be cleaned up
later via host1x_device_del(), so this is not a real leak.
I'll drop this host1x change.
Best regards,
Guangshuo
^ permalink raw reply
* Re: [PATCH v5] mtd: spi-nor: Fix SST AAI write mode opcode handling
From: Sanjaikumar V S @ 2026-04-20 9:02 UTC (permalink / raw)
To: pratyush
Cc: hd, linux-kernel, linux-mtd, miquel.raynal, mwalle, richard,
sanjaikumar.vs, sanjaikumarvs, stable, tudor.ambarus, vigneshr
In-Reply-To: <40364d66-f8a2-4efb-a4d3-70f0aa3137e2@os-cillation.de>
Hi Pratyush,
In v4 you suggested updating dirmap_info with the right opcodes. I went
with a different approach in v5 -- disabling dirmap for SST AAI devices
in sst_nor_late_init() instead. The reasoning is that updating
dirmap_info at runtime is problematic since AAI requires dynamic opcode
and address byte changes per write, and controllers may cache the
template at dirmap_create time.
Hendrik has tested this approach on his SST25VF032B.
Does this approach work for you, or would you prefer a different
direction?
Thanks,
Sanjaikumar
^ permalink raw reply
* [PATCH] fuse: reject oversized dirents in page cache
From: Miklos Szeredi @ 2026-04-20 9:01 UTC (permalink / raw)
To: Christian Brauner; +Cc: Samuel Page, linux-fsdevel, stable, Qi Tang, Zijun Hu
From: Samuel Page <sam@bynar.io>
fuse_add_dirent_to_cache() computes a serialized dirent size from the
server-controlled namelen field and copies the dirent into a single
page-cache page. The existing logic only checks whether the dirent fits
in the remaining space of the current page and advances to a fresh page
if not. It never checks whether the dirent itself exceeds PAGE_SIZE.
As a result, a malicious FUSE server can return a dirent with
namelen=4095, producing a serialized record size of 4120 bytes. On 4 KiB
page systems this causes memcpy() to overflow the cache page by 24 bytes
into the following kernel page.
Reject dirents that cannot fit in a single page before copying them into
the readdir cache.
Fixes: 69e34551152a ("fuse: allow caching readdir")
Cc: stable@vger.kernel.org # v6.16+
Assisted-by: Bynario AI
Signed-off-by: Samuel Page <sam@bynar.io>
Reported-by: Qi Tang <tpluszz77@gmail.com>
Reported-by: Zijun Hu <nightu@northwestern.edu>
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
fs/fuse/readdir.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/fs/fuse/readdir.c b/fs/fuse/readdir.c
index c88194e52d18..db5ae8ec1030 100644
--- a/fs/fuse/readdir.c
+++ b/fs/fuse/readdir.c
@@ -41,6 +41,10 @@ static void fuse_add_dirent_to_cache(struct file *file,
unsigned int offset;
void *addr;
+ /* Dirent doesn't fit in readdir cache page? Skip caching. */
+ if (reclen > PAGE_SIZE)
+ return;
+
spin_lock(&fi->rdc.lock);
/*
* Is cache already completed? Or this entry does not go at the end of
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v2] drm/bridge: dw-hdmi-qp: Guard clear_audio_infoframe when PHY is down
From: Dmitry Baryshkov @ 2026-04-20 9:00 UTC (permalink / raw)
To: Frank Zhang
Cc: andrzej.hajda, neil.armstrong, rfoss, maarten.lankhorst, mripard,
tzimmermann, airlied, simona, detlev.casanova, cristian.ciocaltea,
Laurent.pinchart, jonas, jernej.skrabec, dri-devel, linux-kernel,
stable
In-Reply-To: <9c198d0e-a675-4d7e-a485-5a8ee4d97f88@gmail.com>
On Mon, Apr 20, 2026 at 02:11:28PM +0800, Frank Zhang wrote:
> On 4/19/26 08:40, Dmitry Baryshkov wrote:
> > On Sat, Apr 18, 2026 at 06:19:36PM +0800, Frank Zhang wrote:
> > > The following panic was observed during system reboot:
> > >
> > > Kernel panic - not syncing: Asynchronous SError Interrupt
> > > CPU: 7 UID: 1000 PID: 2637 Comm: pipewire ... 6.19.10-300.fc44.aarch64
> > > Call trace:
> > > ...
> > > regmap_update_bits_base+0x5c/0x90
> > > dw_hdmi_qp_bridge_clear_infoframe+0xb0/0x120 [dw_hdmi_qp]
> > > drm_bridge_connector_clear_infoframe+0x28/0x48 [drm_display_helper]
> > > ...
> > > dw_hdmi_qp_audio_disable+0x24/0xb8 [dw_hdmi_qp]
> > > drm_bridge_connector_audio_shutdown+0x30/0x60 [drm_display_helper]
> > > drm_connector_hdmi_audio_shutdown+0x24/0x38 [drm_display_helper]
> > > hdmi_codec_shutdown+0x60/0x90 [snd_soc_hdmi_codec]
> > > ...
> > > snd_pcm_release_substream.part.0+0x44/0xd8 [snd_pcm]
> > > snd_pcm_release+0x60/0xe8 [snd_pcm]
> > > ...
> > >
> > > The root cause is pipewire tries to close the HDMI audio device after
> > > atomic_disable(), which sets tmds_char_rate to 0 and disable the PHY.
> > >
> > > In this case, dw_hdmi_qp_audio_disable() will call
> > > drm_atomic_helper_connector_hdmi_clear_audio_infoframe() directly,
> > > accessing registers without checking tmds_char_rate.
> > >
> > > Move drm_atomic_helper_connector_hdmi_clear_audio_infoframe() inside the
> > > if (hdmi->tmds_char_rate) of dw_hdmi_qp_audio_disable().
> > >
> > > Fixes: fd0141d1a8a2 ("drm/bridge: synopsys: Add audio support for dw-hdmi-qp")
> > > Signed-off-by: Frank Zhang <rmxpzlb@gmail.com>
> > >
> > > ---
> > > Changes in v2:
> > > - Move drm_atomic_helper_connector_hdmi_clear_audio_infoframe() inside
> > > the if (hdmi->tmds_char_rate) of dw_hdmi_qp_audio_disable().
> > > - Link to v1: https://lore.kernel.org/all/20260416093150.13853-1-rmxpzlb@gmail.com/
> > >
> > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
> > > index d649a1cf07f5..7760527484c8 100644
> > > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
> > > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
> > > @@ -526,10 +526,10 @@ static void dw_hdmi_qp_audio_disable(struct drm_bridge *bridge,
> > > {
> > > struct dw_hdmi_qp *hdmi = dw_hdmi_qp_from_bridge(bridge);
> > > - drm_atomic_helper_connector_hdmi_clear_audio_infoframe(connector);
> > > -
> > > - if (hdmi->tmds_char_rate)
> > > + if (hdmi->tmds_char_rate) {
> > > + drm_atomic_helper_connector_hdmi_clear_audio_infoframe(connector);
> > > dw_hdmi_qp_audio_disable_regs(hdmi);
> > > + }
> >
> > Will audio and audio infoframe remain disabled after consequetive
> > atomic_enable() call?
> >
> > > }
> > > static int dw_hdmi_qp_i2c_read(struct dw_hdmi_qp *hdmi,
> > > --
> > > 2.53.0
> > >
> >
>
> Sorry, I missed clearing the audio infoframe when the PHY is down. The next
> atomic_enable() will write the stale audio infoframe. My mistake.
>
> To clear the stale audio infoframe, dw_hdmi_qp_audio_disable() can handle it
> in the else branch directly, but this seems like a layering violation for a
> bridge driver
>
> I think the better approach is to add a 'reset_audio_infoframe' interface in
> drm_hdmi_state_helper.c that does basically the same as
> drm_atomic_helper_connector_hdmi_clear_audio_infoframe(), but only clearing
> the software state without calling clear_infoframe(). It's also a bit odd
> since it would only be used by dw-hdmi-qp.
That sounds too fine-grained and it also will not work straight ahead...
Just for my understanding, let's consider the opposite situation: the
user tries to start audio playback before setting the mode. How is it
handled in the driver?
>
> I'd like to get the maintainers' opinion about adding such an interface.
>
> Thanks,
> Frank Zhang
>
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH v5] net: caif: fix stack out-of-bounds write in cfctrl_link_setup()
From: Arnd Bergmann @ 2026-04-20 8:14 UTC (permalink / raw)
To: Kangzheng Gu, Simon Horman
Cc: Paolo Abeni, David S . Miller, Eric Dumazet, Jakub Kicinski,
Kees Cook, Thorsten Blum, sjur.brandeland, Netdev, linux-kernel,
stable
In-Reply-To: <CAKvcANPEa91paujTQjpW2hZhpXEhwfOjjy6CsN=OJ32iXYXdTA@mail.gmail.com>
On Mon, Apr 20, 2026, at 10:09, Kangzheng Gu wrote:
> Thanks for all of your advice, I am preparing a new version of patch now.
If you are actively using CAIF, please chime in at
https://lore.kernel.org/all/20260416182829.1440262-1-kuba@kernel.org/
If you are not actually using CAIF, maybe wait a little bit before
spending more time on it because the patches may no longer
apply if it gets removed due to lack of users.
Arnd
^ permalink raw reply
* Re: [PATCH v5] net: caif: fix stack out-of-bounds write in cfctrl_link_setup()
From: Kangzheng Gu @ 2026-04-20 8:09 UTC (permalink / raw)
To: Simon Horman
Cc: Paolo Abeni, davem, edumazet, kuba, kees, thorsten.blum, arnd,
sjur.brandeland, netdev, linux-kernel, stable
In-Reply-To: <20260414112951.GD469338@kernel.org>
Thanks for all of your advice, I am preparing a new version of patch now.
Simon Horman <horms@kernel.org> 于2026年4月14日周二 19:29写道：
>
> On Mon, Apr 13, 2026 at 11:30:53AM +0200, Paolo Abeni wrote:
> > On 4/12/26 3:57 PM, Simon Horman wrote:
> > > I am wondering if it would be best to follow the pattern for
> > > writing linkparam.u.utility.name elsewhere in this function.
> > > That:
> > > 1. Uses a somewhat more succinct loop control structure
> > > 2. Silently truncates input without updating cmdrsp if overrun would occur
> > >
> > > Something like this (compile tested only!):
> > >
> > > diff --git a/net/caif/cfctrl.c b/net/caif/cfctrl.c
> > > index c6cc2bfed65d..ba184c11386e 100644
> > > --- a/net/caif/cfctrl.c
> > > +++ b/net/caif/cfctrl.c
> > > @@ -15,6 +15,7 @@
> > > #include <net/caif/cfctrl.h>
> > >
> > > #define container_obj(layr) container_of(layr, struct cfctrl, serv.layer)
> > > +#define RFM_VOLUME_LEN 20
> > > #define UTILITY_NAME_LENGTH 16
> > > #define CFPKT_CTRL_PKT_LEN 20
> > >
> > > @@ -414,10 +415,11 @@ static int cfctrl_link_setup(struct cfctrl *cfctrl, struct cfpkt *pkt, u8 cmdrsp
> > > */
> > > linkparam.u.rfm.connid = cfpkt_extr_head_u32(pkt);
> > > cp = (u8 *) linkparam.u.rfm.volume;
> > > - for (tmp = cfpkt_extr_head_u8(pkt);
> > > - cfpkt_more(pkt) && tmp != '\0';
> > > - tmp = cfpkt_extr_head_u8(pkt))
> > > + caif_assert(sizeof(linkparam.u.rfm.volume) >= RFM_VOLUME_LEN);
> > > + for(i = 0; i < RFM_VOLUME_LEN - 1 && cfpkt_more(pkt); i++) {
> > > + tmp = cfpkt_extr_head_u8(pkt);
> > > *cp++ = tmp;
> > > + }
> > > *cp = '\0';
> > >
> > > if (CFCTRL_ERR_BIT & cmdrsp)
> >
> > I agree that the code suggested by Simon is clearer. Note that AFAICS it
> > lacks an additional `tmp!= '\0'` check to break the loop, but even with
> > that added it should be preferable.
>
> Sorry, I left out the `tmp!= '\0' check.
> That was unintentional and I agree it should be there.
^ permalink raw reply
* Re: [PATCH v1 2/2] drm/panfrost: Fix wait_bo ioctl leaking positive return from dma_resv_wait_timeout()
From: Boris Brezillon @ 2026-04-20 8:08 UTC (permalink / raw)
To: Gyeyoung Baek
Cc: Tomeu Vizoso, Rob Herring, Steven Price, Adrián Larumbe,
Oded Gabbay, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, dri-devel, linux-kernel, stable
In-Reply-To: <fe33f82fded7be1c18e2e0eb2db451d5a738cf39.1776581974.git.gye976@gmail.com>
On Sun, 19 Apr 2026 16:17:16 +0900
Gyeyoung Baek <gye976@gmail.com> wrote:
> dma_resv_wait_timeout() returns a positive 'remaining jiffies' value
> on success, 0 on timeout, and -errno on failure.
>
> panfrost_ioctl_wait_bo() returns this 'long' result from an int-typed
> ioctl handler, so positive values reach userspace as bogus errors.
> Explicitly set ret to 0 on the success path.
>
> Fixes: f3ba91228e8e ("drm/panfrost: Add initial panfrost driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Gyeyoung Baek <gye976@gmail.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/gpu/drm/panfrost/panfrost_drv.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
> index 3d0bdba2a..784e36d72 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> @@ -390,6 +390,8 @@ panfrost_ioctl_wait_bo(struct drm_device *dev, void *data,
> true, timeout);
> if (!ret)
> ret = timeout ? -ETIMEDOUT : -EBUSY;
> + else if (ret > 0)
> + ret = 0;
>
> drm_gem_object_put(gem_obj);
>
^ permalink raw reply
* Re: [PATCH] arm_pmu: acpi: fix reference leak on failed device registration
From: Greg Kroah-Hartman @ 2026-04-20 8:05 UTC (permalink / raw)
To: Johan Hovold
Cc: Mark Rutland, Guangshuo Li, Will Deacon, Anshuman Khandual,
linux-arm-kernel, linux-perf-users, linux-kernel, stable
In-Reply-To: <aeXVr5enpjb3rfq7@hovoldconsulting.com>
On Mon, Apr 20, 2026 at 09:28:47AM +0200, Johan Hovold wrote:
> On Thu, Apr 16, 2026 at 10:30:23AM +0100, Mark Rutland wrote:
> > On Thu, Apr 16, 2026 at 09:23:33AM +0200, Johan Hovold wrote:
>
> > > It's not just the platform code as this directly reflects the behaviour
> > > of device_register() as Mark pointed out.
> > >
> > > It is indeed an unfortunate quirk of the driver model, but one can argue
> > > that having a registration function that frees its argument on errors
> > > would be even worse. And even more so when many (or most) users get this
> > > right.
> >
> > Ah, sorry; I had missed that the _put() step would actually free the
> > object (and as you explain below, how that won't work for many callers).
> >
> > > So if we want to change this, I think we would need to deprecate
> > > device_register() in favour of explicit device_initialize() and
> > > device_add().
> >
> > Is is possible to have {platfom_,}device_uninitialize() functions that
> > does everything except the ->release() call? If we had that, then we'd
> > be able to have a flow along the lines of:
> >
> > int some_init_function(void)
> > {
> > int err;
> >
> > platform_device_init(&static_pdev);
> >
> > err = platform_device_add(&static_pdev))
> > if (err)
> > goto out_uninit;
> >
> > return 0;
> >
> > out_uninit:
> > platform_device_uninit(&static_pdev);
> > return err;
> > }
> >
> > ... which I think would align with what people generally expect to have
> > to do.
>
> The issue here is that platform_device_add() allocates a device name and
> such resources are not released until the last reference is dropped.
>
> It's been this way since 2008, but some of the static platform devices
> predates that and they both lack a release callback (explicitly required
> since 2003) and are not cleaned up on registration failure.
>
> Since registration would essentially only fail during development (e.g.
> due to name collision or fault injection), this is hardly something to
> worry about, but we could consider moving towards dynamic objects to
> address both issues.
Agreed, this whole thing, including the error handling, is all just
theoretical as no real user ever hits this, which is why it has been
_way_ down my priority list.
> We have a few functions for allocating *and* registering platform
> devices that could be used in many of these cases (and they already
> clean up after themselves on errors):
>
> platform_device_register_simple()
> platform_device_register_data()
> platform_device_register_resndata()
> platform_device_register_full()
>
> and where those do not fit (and cannot be extended) we have the
> underlying:
>
> platform_device_alloc()
> platform_device_add_resources()
> platform_device_add_data()
> plaform_device_add()
>
> But there are some 800 static platform devices left, mostly in legacy
> platform code and board files that I assume few people care about.
Yes, I agree that we do have all of the needed apis here already, we
should just work at converting existing drivers to the new apis OR just
not caring at all as again, no one will ever hit these code paths :)
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v2] dmaengine: Fix refcount leak in channel register error path
From: Guangshuo Li @ 2026-04-20 8:03 UTC (permalink / raw)
To: Frank Li; +Cc: Vinod Koul, Dave Jiang, dmaengine, linux-kernel, stable
In-Reply-To: <aeXGZIrLhqj5hWG8@lizhi-Precision-Tower-5810>
Hi Frank,
Thanks for reviewing.
On Mon, 20 Apr 2026 at 14:23, Frank Li <Frank.li@nxp.com> wrote:
>
>
> I think it is meanless, no one reproduce this. Provide tools link if open
> source. Or you descript how problem happen.
>
The issue was initially reported by a static analysis tool I am developing.
It is still under development and is not open source at this moment.
I also manually reviewed the code path. The problem happens because
device_register() is implemented as:
int device_register(struct device *dev)
{
device_initialize(dev);
return device_add(dev);
}
That means even if device_register() fails, it has already called
device_initialize() and initialized the device reference count to 1.
Also, the comment for device_register() explicitly says:
NOTE: _Never_ directly free @dev after calling this function, even
if it returned an error! Always use put_device() to give up the
reference initialized in this function instead.
In the current code, if device_register(&chan->dev->device) fails, the
error path falls through to:
kfree(chan->dev);
This bypasses the reference-count-based device release path and can lead to
a refcount leak.
> > err_out_ida:
> > ida_free(&device->chan_ida, chan->chan_id);
> > + put_device(&chan->dev->device);
> > + chan->dev = NULL;
> > + goto err_free_local;
>
> avoid err path goto again
>
> >
Thanks for pointing this out. How about this:
err_out_ida:
ida_free(&device->chan_ida, chan->chan_id);
+ put_device(&chan->dev->device);
+ chan->dev = NULL;
+ free_percpu(chan->local);
+ chan->local = NULL;
+ return rc;
+
err_free_dev:
kfree(chan->dev);
err_free_local:
free_percpu(chan->local);
chan->local = NULL;
return rc;
This way, put_device() is only used for the post-device_register()
failure path, while kfree(chan->dev) remains for the earlier failure
path, and the extra goto can be avoided as well.
Thanks,
Guangshuo
^ permalink raw reply
* Re: [PATCH v2 1/4] HID: pass the buffer size to hid_report_raw_event
From: Benjamin Tissoires @ 2026-04-20 8:01 UTC (permalink / raw)
To: Icenowy Zheng
Cc: Jiri Kosina, Filipe Laíns, Bastien Nocera, Ping Cheng,
Jason Gerecke, Viresh Kumar, Johan Hovold, Alex Elder,
Greg Kroah-Hartman, Lee Jones, linux-input, linux-kernel,
greybus-dev, linux-staging, linux-usb, stable
In-Reply-To: <938e8afadcbf2d7b9f0397e24926224985d9c385.camel@icenowy.me>
On Apr 20 2026, Icenowy Zheng wrote:
> 在 2026-04-16四的 16:48 +0200，Benjamin Tissoires写道：
> > commit 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing
> > bogus memset()") enforced the provided data to be at least the size
> > of
> > the declared buffer in the report descriptor to prevent a buffer
> > overflow. However, we can try to be smarter by providing both the
> > buffer
> > size and the data size, meaning that hid_report_raw_event() can make
> > better decision whether we should plaining reject the buffer (buffer
> > overflow attempt) or if we can safely memset it to 0 and pass it to
> > the
> > rest of the stack.
> >
> > Fixes: 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing
> > bogus memset()")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> > ---
> > drivers/hid/bpf/hid_bpf_dispatch.c | 6 ++++--
> > drivers/hid/hid-core.c | 42 +++++++++++++++++++++++++---
> > ----------
> > drivers/hid/hid-gfrm.c | 4 ++--
> > drivers/hid/hid-logitech-hidpp.c | 2 +-
> > drivers/hid/hid-multitouch.c | 2 +-
> > drivers/hid/hid-primax.c | 2 +-
> > drivers/hid/hid-vivaldi-common.c | 2 +-
> > drivers/hid/wacom_sys.c | 6 +++---
> > drivers/staging/greybus/hid.c | 2 +-
> > include/linux/hid.h | 4 ++--
> > include/linux/hid_bpf.h | 14 ++++++++-----
> > 11 files changed, 53 insertions(+), 33 deletions(-)
>
> ============ 8< ===================
>
> > diff --git a/drivers/staging/greybus/hid.c
> > b/drivers/staging/greybus/hid.c
> > index 1f58c907c036..37e8605c6767 100644
> > --- a/drivers/staging/greybus/hid.c
> > +++ b/drivers/staging/greybus/hid.c
> > @@ -201,7 +201,7 @@ static void gb_hid_init_report(struct gb_hid
> > *ghid, struct hid_report *report)
> > * we just need to setup the input fields, so using
> > * hid_report_raw_event is safe.
> > */
> > - hid_report_raw_event(ghid->hid, report->type, ghid->inbuf,
> > size, 1);
> > + hid_report_raw_event(ghid->hid, report->type, ghid->inbuf,
> > ghib->bufsize, size, 1);
>
> Oops, "ghid" is misspelled here...
Damn, you're correct. Sorry.
Jiri, do you want me to send v3? Or can you fix it while applying?
>
> Found this when building some gaint kernel with this patchset.
Thanks a lot for spotting this.
Cheers,
Benjamin
>
> Thanks,
> Icenowy
>
> > }
> >
> > static void gb_hid_init_reports(struct gb_hid *ghid)
>
^ permalink raw reply
* Re: [REGRESSION] Return change in 6.12.80+ with volatile mounting
From: Amir Goldstein @ 2026-04-20 7:59 UTC (permalink / raw)
To: Chenglong Tang; +Cc: Derek Taylor, stable, regressions, Kevin Berry, overlayfs
In-Reply-To: <CAOdxtTaWWu_7eJWu68zf28zHQP3Y--vXTfbGFsceO47BpN3qxA@mail.gmail.com>
On Mon, Apr 20, 2026 at 8:31 AM Chenglong Tang <chenglongtang@google.com> wrote:
>
> Hi, Amir,
>
> Thanks for looking into this! To answer your questions:
>
> 1. Production vs. Test Suite Impact
>
> The immediate failure we encountered is in containerd's integration
> test suite (TestImageVolumeCheckVolatileOption). The test explicitly
> reads /proc/mounts and expects the exact string "volatile".
>
> In default production, containerd passes the legacy "volatile" string
> to the mount syscall, which your patch correctly handles under the
> hood. So the standard "happy path" is not broken in production.
>
> 2. The purpose of WithTempMount() / RemoveVolatileOption
>
> Containerd regularly makes temporary overlay mounts (e.g., for
> unpacking layers). Because overlayfs rejects reusing upper/work dirs
> from a volatile mount, containerd uses RemoveVolatileOption to strip
> the volatile flag before these temporary mounts.
>
> Currently, containerd's RemoveVolatileOption does an exact string
> match for "volatile". While it works for the default path, there is a
> production edge case: if a user explicitly configures their container
> runtime to use the new "fsync=volatile" option, older containerd
> binaries will fail to strip it, and the temporary mounts will be
> rejected by the kernel.
I need to challenge this specific argument because I do not agree
that this edge case could be considered a regression at all.
An admin from the past could not have set an explicit
"fsync=volatile" mount option.
Though experiment - overlayfs adds a new mode fsync=off
which is more loose than "volatile" because "volatile" can actually
return error on fsync in some cases.
Overlayfs would also not allow to reuse workdir from such
a mount.
So would you then claim that adding the new mount option
"fsync=off" is a regression because of the edge case that an admin
decided to explicitly add "fsync=off" and RemoveVolatileOption()
does not handle it.
I don't buy it.
There is a more correct way to handle this situation and it is
documented in overlayfs.rst:
"When overlay is mounted with "volatile" option, the directory
"$workdir/work/incompat/volatile" is created. During next mount, overlay
checks for this directory and refuses to mount if present. This is a strong
indicator that the user should discard upper and work directories and create
fresh ones. In very limited cases where the user knows that the system has
not crashed and contents of upperdir are intact, the "volatile" directory
can be removed."
containerd unpacking layers falls under this very limited case.
containerd can syncfs() workdir after unpack & unmount of temp
overlayfs and remove the "incompat/volatile" directory and then
the upperdir/workdir are are free to be remounted.
>
> Conclusion
>
> While containerd could theoretically patch their code to accept
> strings.Contains() or fsync=volatile going forward, there are many
Following the documentation advise would be better.
> existing containerd binaries in the wild. Given that this patch breaks
> containerd's CI tests and introduces an edge case for
> RemoveVolatileOption, it might be safest to fix ovl_show_options in
> the kernel to continue outputting the legacy "volatile" string to
> strictly guarantee backwards compatibility with userspace.
>
Considering my comments above, I think we have not yet reached
the point where the backward compat "volatile" string is called for,
but with other reports from production workloads, we could get there.
Thanks for the clear and honest explanations of the containerd situation!
Amir.
^ permalink raw reply
* Re: [PATCH 0/4] media: uvcvideo: Fixes for hw timestamping
From: Yunke Cao @ 2026-04-20 7:38 UTC (permalink / raw)
To: Ricardo Ribalda
Cc: Laurent Pinchart, Hans de Goede, Mauro Carvalho Chehab,
Tomasz Figa, Sergey Senozhatsky, linux-media, linux-kernel,
stable
In-Reply-To: <20260323-uvc-hwtimestamp-v1-0-aa42e3865204@chromium.org>
Hi Ricardo,
I tested the series on a SunplusIT Inc 1080p FHD Camera (2b7e:c877).
Without this series, hardware timestamping was broken (due to the
issue fixed by [PATCH 2/4] of this series).
With this series, hardware timestamping works as intended.
Tested-by: Yunke Cao <yunkec@google.com>
Best,
Yunke
On Mon, Mar 23, 2026 at 9:10 PM Ricardo Ribalda <ribalda@chromium.org> wrote:
>
> This series introduces fixes for the hardware timestamp calculations.
>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
> Ricardo Ribalda (4):
> media: uvcvideo: Fix dev_sof filtering in hw timestamp
> media: uvcvideo: Use hw timestaming if the clock buffer is full
> media: uvcvideo: Relax the constrains for interpolating the hw clock
> media: uvcvideo: Do not add clock samples with small sof delta
>
> drivers/media/usb/uvc/uvc_video.c | 51 +++++++++++++++++++++++++++------------
> 1 file changed, 35 insertions(+), 16 deletions(-)
> ---
> base-commit: a7da7fb57f2a787412da1a62292a17fa00fbfbdf
> change-id: 20260309-uvc-hwtimestamp-f25dc27f5711
>
> Best regards,
> --
> Ricardo Ribalda <ribalda@chromium.org>
>
^ permalink raw reply
* Re: [PATCH v2 1/2] hwtracing: hisi_ptt: Propagate DMA reset timeout in trace_start()
From: Sizhe Liu @ 2026-04-20 7:32 UTC (permalink / raw)
To: Yicong Yang, Pradhan, Sanman, jonathan.cameron@huawei.com
Cc: alexander.shishkin@linux.intel.com, linux-kernel@vger.kernel.org,
stable@vger.kernel.org, Sanman Pradhan, Suzuki K Poulose,
Jie Zhan
In-Reply-To: <166964d1-2d16-43e8-b4d7-27b4a5e6286d@gmail.com>
On 2026/4/17 1:25, Yicong Yang wrote:
> +cc Suzuki and Sizhe..
>
> On 2026/4/15 01:25, Pradhan, Sanman wrote:
>> From: Sanman Pradhan <psanman@juniper.net>
>>
>> hisi_ptt_wait_dma_reset_done() discards the return value of
>> readl_poll_timeout_atomic(). If the DMA engine does not complete its
>> reset within the timeout, hisi_ptt_trace_start() proceeds to start
>> tracing regardless.
>>
>> Return a bool from hisi_ptt_wait_dma_reset_done(), consistent with the
>> other wait helpers in this driver. On timeout, log an error, de-assert
>> the reset bit, and return -ETIMEDOUT. Move ctrl->started to the
>> successful path so a failed start does not leave the trace marked as
>> active.
>>
>> Fixes: ff0de066b463 ("hwtracing: hisi_ptt: Add trace function support for HiSilicon PCIe Tune and Trace device")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Sanman Pradhan <psanman@juniper.net>
> looks good to me.
>
> Reviewed-by: Yicong Yang <yangyccccc@gmail.com>
>
> I see the Suzuki has sent out the PR for 7.1, so this may wait after the merge window...
>
> thanks.
Looks good to me, thanks.
Reviewed-by: Sizhe Liu <liusizhe5@huawei.com>
>> ---
>> v2:
>> - Return bool for consistency with other wait helpers
>> - Add pci_err() on timeout
>> - De-assert RST before returning on timeout
>> - Move ctrl->started to the successful path
>>
>> drivers/hwtracing/ptt/hisi_ptt.c | 20 +++++++++++++-------
>> 1 file changed, 13 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/hwtracing/ptt/hisi_ptt.c b/drivers/hwtracing/ptt/hisi_ptt.c
>> index 94c371c491357..b5d851281fbf0 100644
>> --- a/drivers/hwtracing/ptt/hisi_ptt.c
>> +++ b/drivers/hwtracing/ptt/hisi_ptt.c
>> @@ -171,13 +171,13 @@ static bool hisi_ptt_wait_trace_hw_idle(struct hisi_ptt *hisi_ptt)
>> HISI_PTT_WAIT_TRACE_TIMEOUT_US);
>> }
>>
>> -static void hisi_ptt_wait_dma_reset_done(struct hisi_ptt *hisi_ptt)
>> +static bool hisi_ptt_wait_dma_reset_done(struct hisi_ptt *hisi_ptt)
>> {
>> u32 val;
>>
>> - readl_poll_timeout_atomic(hisi_ptt->iobase + HISI_PTT_TRACE_WR_STS,
>> - val, !val, HISI_PTT_RESET_POLL_INTERVAL_US,
>> - HISI_PTT_RESET_TIMEOUT_US);
>> + return !readl_poll_timeout_atomic(hisi_ptt->iobase + HISI_PTT_TRACE_WR_STS,
>> + val, !val, HISI_PTT_RESET_POLL_INTERVAL_US,
>> + HISI_PTT_RESET_TIMEOUT_US);
>> }
>>
>> static void hisi_ptt_trace_end(struct hisi_ptt *hisi_ptt)
>> @@ -202,14 +202,18 @@ static int hisi_ptt_trace_start(struct hisi_ptt *hisi_ptt)
>> return -EBUSY;
>> }
>>
>> - ctrl->started = true;
>> -
>> /* Reset the DMA before start tracing */
>> val = readl(hisi_ptt->iobase + HISI_PTT_TRACE_CTRL);
>> val |= HISI_PTT_TRACE_CTRL_RST;
>> writel(val, hisi_ptt->iobase + HISI_PTT_TRACE_CTRL);
>>
>> - hisi_ptt_wait_dma_reset_done(hisi_ptt);
>> + if (!hisi_ptt_wait_dma_reset_done(hisi_ptt)) {
>> + pci_err(hisi_ptt->pdev, "timed out waiting for DMA reset\n");
>> + val = readl(hisi_ptt->iobase + HISI_PTT_TRACE_CTRL);
>> + val &= ~HISI_PTT_TRACE_CTRL_RST;
>> + writel(val, hisi_ptt->iobase + HISI_PTT_TRACE_CTRL);
>> + return -ETIMEDOUT;
>> + }
>>
>> val = readl(hisi_ptt->iobase + HISI_PTT_TRACE_CTRL);
>> val &= ~HISI_PTT_TRACE_CTRL_RST;
>> @@ -234,6 +238,8 @@ static int hisi_ptt_trace_start(struct hisi_ptt *hisi_ptt)
>> if (!hisi_ptt->trace_ctrl.is_port)
>> val |= HISI_PTT_TRACE_CTRL_FILTER_MODE;
>>
>> + ctrl->started = true;
>> +
>> /* Start the Trace */
>> val |= HISI_PTT_TRACE_CTRL_EN;
>> writel(val, hisi_ptt->iobase + HISI_PTT_TRACE_CTRL);
^ permalink raw reply
* Re: [PATCH] arm_pmu: acpi: fix reference leak on failed device registration
From: Johan Hovold @ 2026-04-20 7:28 UTC (permalink / raw)
To: Mark Rutland
Cc: Greg Kroah-Hartman, Guangshuo Li, Will Deacon, Anshuman Khandual,
linux-arm-kernel, linux-perf-users, linux-kernel, stable
In-Reply-To: <aeCsLy-45QyeCwGA@J2N7QTR9R3>
On Thu, Apr 16, 2026 at 10:30:23AM +0100, Mark Rutland wrote:
> On Thu, Apr 16, 2026 at 09:23:33AM +0200, Johan Hovold wrote:
> > It's not just the platform code as this directly reflects the behaviour
> > of device_register() as Mark pointed out.
> >
> > It is indeed an unfortunate quirk of the driver model, but one can argue
> > that having a registration function that frees its argument on errors
> > would be even worse. And even more so when many (or most) users get this
> > right.
>
> Ah, sorry; I had missed that the _put() step would actually free the
> object (and as you explain below, how that won't work for many callers).
>
> > So if we want to change this, I think we would need to deprecate
> > device_register() in favour of explicit device_initialize() and
> > device_add().
>
> Is is possible to have {platfom_,}device_uninitialize() functions that
> does everything except the ->release() call? If we had that, then we'd
> be able to have a flow along the lines of:
>
> int some_init_function(void)
> {
> int err;
>
> platform_device_init(&static_pdev);
>
> err = platform_device_add(&static_pdev))
> if (err)
> goto out_uninit;
>
> return 0;
>
> out_uninit:
> platform_device_uninit(&static_pdev);
> return err;
> }
>
> ... which I think would align with what people generally expect to have
> to do.
The issue here is that platform_device_add() allocates a device name and
such resources are not released until the last reference is dropped.
It's been this way since 2008, but some of the static platform devices
predates that and they both lack a release callback (explicitly required
since 2003) and are not cleaned up on registration failure.
Since registration would essentially only fail during development (e.g.
due to name collision or fault injection), this is hardly something to
worry about, but we could consider moving towards dynamic objects to
address both issues.
We have a few functions for allocating *and* registering platform
devices that could be used in many of these cases (and they already
clean up after themselves on errors):
platform_device_register_simple()
platform_device_register_data()
platform_device_register_resndata()
platform_device_register_full()
and where those do not fit (and cannot be extended) we have the
underlying:
platform_device_alloc()
platform_device_add_resources()
platform_device_add_data()
plaform_device_add()
But there are some 800 static platform devices left, mostly in legacy
platform code and board files that I assume few people care about.
Johan
^ permalink raw reply
* Re: [f2fs-dev] [PATCH] f2fs: fix node_cnt race between extent node destroy and writeback
From: Chao Yu @ 2026-04-20 7:28 UTC (permalink / raw)
To: Yongpeng Yang, Jaegeuk Kim; +Cc: chao, Yongpeng Yang, stable, linux-f2fs-devel
In-Reply-To: <a643b967-cb05-4de5-96f2-f1b783c9758d@sina.com>
On 4/19/2026 12:29 AM, Yongpeng Yang wrote:
>
> On 4/18/26 8:51 AM, Chao Yu via Linux-f2fs-devel wrote:
>> On 4/17/26 21:26, Yongpeng Yang wrote:
>>>
>>> On 4/17/26 17:00, Chao Yu via Linux-f2fs-devel wrote:
>>>> On 4/3/26 22:40, Yongpeng Yang wrote:
>>>>> From: Yongpeng Yang <yangyongpeng@xiaomi.com>
>>>>>
>>>>> f2fs_destroy_extent_node() does not set FI_NO_EXTENT before clearing
>>>>> extent nodes. When called from f2fs_drop_inode() with I_SYNC set,
>>>>> concurrent kworker writeback can insert new extent nodes into the same
>>>>> extent tree, racing with the destroy and triggering f2fs_bug_on() in
>>>>> __destroy_extent_node(). The scenario is as follows:
>>>>>
>>>>> drop inode writeback
>>>>> - iput
>>>>> - f2fs_drop_inode // I_SYNC set
>>>>> - f2fs_destroy_extent_node
>>>>> - __destroy_extent_node
>>>>> - while (node_cnt) {
>>>>> write_lock(&et->lock)
>>>>> __free_extent_tree
>>>>> write_unlock(&et->lock)
>>>>> - __writeback_single_inode
>>>>> - f2fs_outplace_write_data
>>>>> -
>>>>> f2fs_update_read_extent_cache
>>>>> -
>>>>> __update_extent_tree_range
>>>>> // FI_NO_EXTENT not set,
>>>>> // insert new extent node
>>>>> } // node_cnt == 0, exit while
>>>>> - f2fs_bug_on(node_cnt) // node_cnt > 0
>>>>>
>>>>> Additionally, __update_extent_tree_range() only checks FI_NO_EXTENT for
>>>>> EX_READ type, leaving EX_BLOCK_AGE updates completely unprotected.
>>>>>
>>>>> This patch set FI_NO_EXTENT under et->lock in __destroy_extent_node(),
>>>>> consistent with other callers (__update_extent_tree_range and
>>>>> __drop_extent_tree) and check FI_NO_EXTENT for both EX_READ and
>>>>> EX_BLOCK_AGE tree.
>>>>
>>>> I suffered below test failure, then I bisect to this change.
>>>>
>>>> generic/475 84s ... [failed, exit status 1]- output mismatch
>>>> (see /
>>>> share/git/fstests/results//generic/475.out.bad)
>>>> --- tests/generic/475.out 2025-01-12 21:57:40.279440664 +0800
>>>> +++ /share/git/fstests/results//generic/475.out.bad 2026-04-17
>>>> 12:08:28.000000000 +0800
>>>> @@ -1,2 +1,6 @@
>>>> QA output created by 475
>>>> Silence is golden.
>>>> +mount: /mnt/scratch_f2fs: mount system call failed: Structure
>>>> needs
>>>> cleaning.
>>>> + dmesg(1) may have more information after failed mount
>>>> system
>>>> call.
>>>> +mount failed
>>>> +(see /share/git/fstests/results//generic/475.full for details)
>>>> ...
>>>> (Run 'diff -u /share/git/fstests/tests/generic/475.out /share/git/
>>>> fstests/results//generic/475.out.bad' to see the entire diff)
>>>>
>>>>
>>>> generic/388 73s ... [failed, exit status 1]- output mismatch
>>>> (see /
>>>> share/git/fstests/results//generic/388.out.bad)
>>>> --- tests/generic/388.out 2025-01-12 21:57:40.275440602 +0800
>>>> +++ /share/git/fstests/results//generic/388.out.bad 2026-04-17
>>>> 11:58:05.000000000 +0800
>>>> @@ -1,2 +1,6 @@
>>>> QA output created by 388
>>>> Silence is golden.
>>>> +mount: /mnt/scratch_f2fs: mount system call failed: Structure
>>>> needs
>>>> cleaning.
>>>> + dmesg(1) may have more information after failed mount
>>>> system
>>>> call.
>>>> +cycle mount failed
>>>> +(see /share/git/fstests/results//generic/388.full for details)
>>>> ...
>>>> (Run 'diff -u /share/git/fstests/tests/generic/388.out /share/git/
>>>> fstests/results//generic/388.out.bad' to see the entire diff)
>>>>
>>>>
>>>> F2FS-fs (dm-0): sanity_check_extent_cache: inode (ino=1761) extent
>>>> info [220057, 57, 6] is incorrect, run fsck to fix
>>>>
>>>> I suspect we may miss any extent updates after we set FI_NO_EXTENT in
>>>> __destroy_extent_node(), result in failing in
>>>> sanity_check_extent_cache().
>>>>
>>>> Can we just relocate f2fs_bug_on(node_cnt) rather than complicated
>>>> change?
>>>> Thoughts?
>>>
>>> Oh, I overlooked largest extent. How about relocate
>>> f2fs_bug_on(node_cnt) to __destroy_extent_tree?
>>>
>>> static void __destroy_extent_tree(struct inode *inode, enum extent_type
>>> type)
>>>
>>> /* free all extent info belong to this extent tree */
>>> node_cnt = __destroy_extent_node(inode, type);
>>> + f2fs_bug_on(sbi, atomic_read(&et->node_cnt));
>>
>> /* free all extent info belong to this extent tree */
>> node_cnt = __destroy_extent_node(inode, type);
>>
>> /* delete extent tree entry in radix tree */
>> mutex_lock(&eti->extent_tree_lock);
>> f2fs_bug_on(sbi, atomic_read(&et->node_cnt)); <---
>>
>> Oh, it has already checked node_cnt, so, maybe we can just remove the
>> check in
>> __destroy_extent_node()?
>
> Yes. BTW, is it correct to remove the call to f2fs_destroy_extent_node()
> in f2fs_drop_inode()? It seems this call is unnecessary, since
> f2fs_evict_inode() will eventually delete all extent nodes properly.
I think it's fine to keep it according to original intention "destroy
extent_tree for the truncation case" introduced from 3e72f721390d
("f2fs: use extent_cache by default"). It helps the performance w/
in batch extent node release.
Thanks,
>
> Thanks
> Yongpeng,
>
>>
>> Thanks,
>>
>>
>>>
>>> Thanks
>>> Yongpeng,
>>>
>>>>
>>>> Thanks,
>>>>
>>>>>
>>>>> Fixes: 3fc5d5a182f6 ("f2fs: fix to shrink read extent node in batches")
>>>>> Cc: stable@vger.kernel.org
>>>>> Signed-off-by: Yongpeng Yang <yangyongpeng@xiaomi.com>
>>>>> ---
>>>>> fs/f2fs/extent_cache.c | 17 ++++++++++-------
>>>>> 1 file changed, 10 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
>>>>> index 0ed84cc065a7..87169fd29d89 100644
>>>>> --- a/fs/f2fs/extent_cache.c
>>>>> +++ b/fs/f2fs/extent_cache.c
>>>>> @@ -119,9 +119,10 @@ static bool __may_extent_tree(struct inode
>>>>> *inode, enum extent_type type)
>>>>> if (!__init_may_extent_tree(inode, type))
>>>>> return false;
>>>>> + if (is_inode_flag_set(inode, FI_NO_EXTENT))
>>>>> + return false;
>>>>> +
>>>>> if (type == EX_READ) {
>>>>> - if (is_inode_flag_set(inode, FI_NO_EXTENT))
>>>>> - return false;
>>>>> if (is_inode_flag_set(inode, FI_COMPRESSED_FILE) &&
>>>>> !f2fs_sb_has_readonly(F2FS_I_SB(inode)))
>>>>> return false;
>>>>> @@ -644,6 +645,8 @@ static unsigned int __destroy_extent_node(struct
>>>>> inode *inode,
>>>>> while (atomic_read(&et->node_cnt)) {
>>>>> write_lock(&et->lock);
>>>>> + if (!is_inode_flag_set(inode, FI_NO_EXTENT))
>>>>> + set_inode_flag(inode, FI_NO_EXTENT);
>>>>> node_cnt += __free_extent_tree(sbi, et, nr_shrink);
>>>>> write_unlock(&et->lock);
>>>>> }
>>>>> @@ -688,12 +691,12 @@ static void __update_extent_tree_range(struct
>>>>> inode *inode,
>>>>> write_lock(&et->lock);
>>>>> - if (type == EX_READ) {
>>>>> - if (is_inode_flag_set(inode, FI_NO_EXTENT)) {
>>>>> - write_unlock(&et->lock);
>>>>> - return;
>>>>> - }
>>>>> + if (is_inode_flag_set(inode, FI_NO_EXTENT)) {
>>>>> + write_unlock(&et->lock);
>>>>> + return;
>>>>> + }
>>>>> + if (type == EX_READ) {
>>>>> prev = et->largest;
>>>>> dei.len = 0;
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> Linux-f2fs-devel mailing list
>>>> Linux-f2fs-devel@lists.sourceforge.net
>>>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>>>
>>
>>
>>
>> _______________________________________________
>> Linux-f2fs-devel mailing list
>> Linux-f2fs-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>
^ permalink raw reply
* Re: [PATCH v2] gpu: host1x: Fix device reference leak in device_add() error path
From: Mikko Perttunen @ 2026-04-20 7:19 UTC (permalink / raw)
To: Thierry Reding, David Airlie, Simona Vetter, Guangshuo Li,
Vamsee Vardhan Thummala, linux-kernel, dri-devel, linux-tegra,
Guangshuo Li
Cc: stable
In-Reply-To: <20260413141328.2954939-1-lgs201920130244@gmail.com>
On Monday, April 13, 2026 11:13 PM Guangshuo Li wrote:
> After device_initialize(), the embedded struct device in struct
> host1x_device should be released through the device core with
> put_device().
>
> In host1x_device_add(), the empty-subdevice path calls
> device_add(&device->dev), but if that fails it only logs the error and
> continues without dropping the device reference. That leaks the
> reference held on the embedded struct device.
>
> The issue was identified by a static analysis tool I developed and
> confirmed by manual review.
>
> Fix this by removing the device from host1x->devices and calling
> put_device() when device_add() fails.
>
> Fixes: fab823d82ee50 ("gpu: host1x: Allow loading tegra-drm without enabled engines")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
> v2:
> - note that the issue was identified by my static analysis tool
> - and confirmed by manual review
>
> drivers/firmware/edd.c | 2 +-
> drivers/gpu/host1x/bus.c | 6 +++++-
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/firmware/edd.c b/drivers/firmware/edd.c
> index 55dec4eb2c00..82b326ce83ce 100644
> --- a/drivers/firmware/edd.c
> +++ b/drivers/firmware/edd.c
> @@ -748,7 +748,7 @@ edd_init(void)
>
> rc = edd_device_register(edev, i);
> if (rc) {
> - kfree(edev);
> + kobject_put(&edev->kobj);
> goto out;
> }
> edd_devices[i] = edev;
Unrelated ..
> diff --git a/drivers/gpu/host1x/bus.c b/drivers/gpu/host1x/bus.c
> index 723a80895cd4..63fe037c3b65 100644
> --- a/drivers/gpu/host1x/bus.c
> +++ b/drivers/gpu/host1x/bus.c
> @@ -477,8 +477,12 @@ static int host1x_device_add(struct host1x *host1x,
> */
> if (list_empty(&device->subdevs)) {
> err = device_add(&device->dev);
> - if (err < 0)
> + if (err < 0) {
> dev_err(&device->dev, "failed to add device: %d\n", err);
> + list_del(&device->list);
> + put_device(&device->dev);
> + return err;
> + }
> else
> device->registered = true;
This isn't a leak -- if device_add fails, the device is still on the
device list, though in a "stuck" state, and will get cleaned up through
host1x_device_del.
Thanks
Mikko
> }
> --
> 2.43.0
>
>
^ permalink raw reply
* Re: [PATCH] drm/bridge: imx8qxp-pxl2dpi: avoid of_node_put() on ERR_PTR()
From: Liu Ying @ 2026-04-20 6:53 UTC (permalink / raw)
To: Guangshuo Li, Frank Li
Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Luca Ceresoli, dri-devel,
imx, linux-arm-kernel, linux-kernel, stable
In-Reply-To: <CANUHTR8FaXLX+Nbeb7+sWRF9jQ5SoBgWc2y_LVD38KE7TqsxeQ@mail.gmail.com>
On Mon, Apr 20, 2026 at 10:19:35AM +0800, Guangshuo Li wrote:
> [You don't often get email from lgs201920130244@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> Hi Frank,
>
> Thanks for the review.
>
> On Mon, 20 Apr 2026 at 09:56, Frank Li <Frank.li@nxp.com> wrote:
>>
>>
>> Please fix
>> DEFINE_FREE(device_node, struct device_node *, if (_T) of_node_put(_T))
>>
>> If (!IS_ERR(_T))
>>
>
> You're right, fixing DEFINE_FREE(device_node, ...) is the proper way
> to handle this:
> if (_T && !IS_ERR(_T)) of_node_put(_T)
This would be intrusive because it effectively changes the cleanup action.
A similar case[1] was handled by ensuring only NULL pointer was returned
on error. And, this is actually what i2c_of_probe_get_i2c_node()[2] does
now.
[1] https://lore.kernel.org/all/Zw-VkQ3di5nFHiXB@smile.fi.intel.com/
[2] https://elixir.bootlin.com/linux/v7.0/source/drivers/i2c/i2c-core-of-prober.c#L38-L58
BTW, even if the cleanup action needs to be changed, the 'if' condition
should be '!IS_ERR_OR_NULL(_T)'.
>
> This is a better fix than handling it only in this driver.
>
> I'll rework the patch based on your suggestion and send v2 later.
>
> Thanks,
> Guangshuo
--
Regards,
Liu Ying
^ permalink raw reply
* [PATCH 05/38] crypto: drbg - Fix the fips_enabled priority boost
From: Eric Biggers @ 2026-04-20 6:33 UTC (permalink / raw)
To: linux-crypto, Herbert Xu
Cc: linux-kernel, Stephan Mueller, Jason A . Donenfeld, Eric Biggers,
stable
In-Reply-To: <20260420063422.324906-1-ebiggers@kernel.org>
When fips_enabled=1, it seems to have been intended for one of the
algorithms defined in crypto/drbg.c to be the highest priority "stdrng"
algorithm, so that it is what is used by "stdrng" users.
However, the code only boosts the priority to 400, which is less than
the priority 500 used in drivers/crypto/caam/caamprng.c. Thus, the CAAM
RNG could be used instead.
Fix this by boosting the priority by 2000 instead of 200.
Fixes: 541af946fe13 ("crypto: drbg - SP800-90A Deterministic Random Bit Generator")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
crypto/drbg.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/crypto/drbg.c b/crypto/drbg.c
index f23b431bd490..e3065fb9541b 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -1830,11 +1830,11 @@ static inline void __init drbg_fill_array(struct rng_alg *alg,
* If FIPS mode enabled, the selected DRBG shall have the
* highest cra_priority over other stdrng instances to ensure
* it is selected.
*/
if (fips_enabled)
- alg->base.cra_priority += 200;
+ alg->base.cra_priority += 2000;
alg->base.cra_ctxsize = sizeof(struct drbg_state);
alg->base.cra_module = THIS_MODULE;
alg->base.cra_init = drbg_kcapi_init;
alg->base.cra_exit = drbg_kcapi_cleanup;
--
2.53.0
^ permalink raw reply related
* [PATCH 04/38] crypto: drbg - Fix drbg_max_addtl() on 64-bit kernels
From: Eric Biggers @ 2026-04-20 6:33 UTC (permalink / raw)
To: linux-crypto, Herbert Xu
Cc: linux-kernel, Stephan Mueller, Jason A . Donenfeld, Eric Biggers,
stable
In-Reply-To: <20260420063422.324906-1-ebiggers@kernel.org>
On 64-bit kernels, drbg_max_addtl() returns 2**35 bytes. That's too
large, for two reasons:
1. SP800-90A says the maximum limit is 2**35 *bits*, not 2**35 bytes.
So the implemented limit has confused bits and bytes.
2. When drbg_kcapi_hash() calls crypto_shash_update() on the additional
information string, the length is implicitly cast to 'unsigned int'.
That truncates the additional information string to U32_MAX bytes.
Fix the maximum additional information string length to always be
U32_MAX - 1, causing an error to be returned for any longer lengths.
Fixes: 541af946fe13 ("crypto: drbg - SP800-90A Deterministic Random Bit Generator")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
include/crypto/drbg.h | 18 +++++++-----------
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/include/crypto/drbg.h b/include/crypto/drbg.h
index 2d42518cbdce..c11eaf757ed0 100644
--- a/include/crypto/drbg.h
+++ b/include/crypto/drbg.h
@@ -146,23 +146,19 @@ static inline size_t drbg_max_request_bytes(struct drbg_state *drbg)
{
/* SP800-90A requires the limit 2**19 bits, but we return bytes */
return (1 << 16);
}
+/*
+ * SP800-90A allows implementations to support additional info / personalization
+ * strings of up to 2**35 bits. Implementations can have a smaller maximum. We
+ * use 2**35 - 16 bits == U32_MAX - 1 bytes so that the max + 1 always fits in a
+ * size_t, allowing drbg_healthcheck_sanity() to verify its enforcement.
+ */
static inline size_t drbg_max_addtl(struct drbg_state *drbg)
{
- /* SP800-90A requires 2**35 bytes additional info str / pers str */
-#if (__BITS_PER_LONG == 32)
- /*
- * SP800-90A allows smaller maximum numbers to be returned -- we
- * return SIZE_MAX - 1 to allow the verification of the enforcement
- * of this value in drbg_healthcheck_sanity.
- */
- return (SIZE_MAX - 1);
-#else
- return (1UL<<35);
-#endif
+ return U32_MAX - 1;
}
static inline size_t drbg_max_requests(struct drbg_state *drbg)
{
/* SP800-90A requires 2**48 maximum requests before reseeding */
--
2.53.0
^ permalink raw reply related
* [PATCH 03/38] crypto: drbg - Fix ineffective sanity check
From: Eric Biggers @ 2026-04-20 6:33 UTC (permalink / raw)
To: linux-crypto, Herbert Xu
Cc: linux-kernel, Stephan Mueller, Jason A . Donenfeld, Eric Biggers,
stable
In-Reply-To: <20260420063422.324906-1-ebiggers@kernel.org>
Fix drbg_healthcheck_sanity() to correctly check the return value of
drbg_generate(). drbg_generate() returns 0 on success, or a negative
errno value on failure. drbg_healthcheck_sanity() incorrectly assumed
that it returned a positive value on success.
This didn't make the sanity check fail, but it made it ineffective.
Fixes: cde001e4c3c3 ("crypto: rng - RNGs must return 0 in success case")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
crypto/drbg.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/crypto/drbg.c b/crypto/drbg.c
index de4c69032155..f23b431bd490 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -1735,11 +1735,10 @@ static int drbg_kcapi_seed(struct crypto_rng *tfm,
* Note 2: There is no sensible way of testing the reseed counter
* enforcement, so skip it.
*/
static inline int __init drbg_healthcheck_sanity(void)
{
- int len = 0;
#define OUTBUFLEN 16
unsigned char buf[OUTBUFLEN];
struct drbg_state *drbg = NULL;
int ret;
int rc = -EFAULT;
@@ -1780,15 +1779,15 @@ static inline int __init drbg_healthcheck_sanity(void)
max_addtllen = drbg_max_addtl(drbg);
max_request_bytes = drbg_max_request_bytes(drbg);
drbg_string_fill(&addtl, buf, max_addtllen + 1);
/* overflow addtllen with additional info string */
- len = drbg_generate(drbg, buf, OUTBUFLEN, &addtl);
- BUG_ON(0 < len);
+ ret = drbg_generate(drbg, buf, OUTBUFLEN, &addtl);
+ BUG_ON(ret == 0);
/* overflow max_bits */
- len = drbg_generate(drbg, buf, (max_request_bytes + 1), NULL);
- BUG_ON(0 < len);
+ ret = drbg_generate(drbg, buf, max_request_bytes + 1, NULL);
+ BUG_ON(ret == 0);
/* overflow max addtllen with personalization string */
ret = drbg_seed(drbg, &addtl, false);
BUG_ON(0 == ret);
/* all tests passed */
--
2.53.0
^ permalink raw reply related
* [PATCH 02/38] crypto: drbg - Fix misaligned writes in CTR_DRBG and HASH_DRBG
From: Eric Biggers @ 2026-04-20 6:33 UTC (permalink / raw)
To: linux-crypto, Herbert Xu
Cc: linux-kernel, Stephan Mueller, Jason A . Donenfeld, Eric Biggers,
stable
In-Reply-To: <20260420063422.324906-1-ebiggers@kernel.org>
drbg_cpu_to_be32() is being used to do a plain write to a byte array,
which doesn't have any alignment guarantee. This can cause a misaligned
write. Replace it with the correct function, put_unaligned_be32().
Fixes: 72f3e00dd67e ("crypto: drbg - replace int2byte with cpu_to_be")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
crypto/df_sp80090a.c | 7 ++++---
crypto/drbg.c | 3 ++-
include/crypto/internal/drbg.h | 18 ------------------
3 files changed, 6 insertions(+), 22 deletions(-)
diff --git a/crypto/df_sp80090a.c b/crypto/df_sp80090a.c
index b8134be6f7ad..f4bb7be016e8 100644
--- a/crypto/df_sp80090a.c
+++ b/crypto/df_sp80090a.c
@@ -8,10 +8,11 @@
#include <linux/errno.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/string.h>
+#include <linux/unaligned.h>
#include <crypto/aes.h>
#include <crypto/df_sp80090a.h>
#include <crypto/internal/drbg.h>
static void drbg_kcapi_sym(struct aes_enckey *aeskey, unsigned char *outval,
@@ -139,14 +140,14 @@ int crypto_drbg_ctr_df(struct aes_enckey *aeskey,
return -EINVAL;
/* 10.4.2 step 2 -- calculate the entire length of all input data */
list_for_each_entry(seed, seedlist, list)
inputlen += seed->len;
- drbg_cpu_to_be32(inputlen, &L_N[0]);
+ put_unaligned_be32(inputlen, &L_N[0]);
/* 10.4.2 step 3 */
- drbg_cpu_to_be32(bytes_to_return, &L_N[4]);
+ put_unaligned_be32(bytes_to_return, &L_N[4]);
/* 10.4.2 step 5: length is L_N, input_string, one byte, padding */
padlen = (inputlen + sizeof(L_N) + 1) % (blocklen_bytes);
/* wrap the padlen appropriately */
if (padlen)
@@ -173,11 +174,11 @@ int crypto_drbg_ctr_df(struct aes_enckey *aeskey,
/*
* 10.4.2 step 9.1 - the padding is implicit as the buffer
* holds zeros after allocation -- even the increment of i
* is irrelevant as the increment remains within length of i
*/
- drbg_cpu_to_be32(i, iv);
+ put_unaligned_be32(i, iv);
/* 10.4.2 step 9.2 -- BCC and concatenation with temp */
drbg_ctr_bcc(aeskey, temp + templen, K, &bcc_list,
blocklen_bytes, keylen);
/* 10.4.2 step 9.3 */
i++;
diff --git a/crypto/drbg.c b/crypto/drbg.c
index e4eb78ed222b..de4c69032155 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -101,10 +101,11 @@
#include <crypto/df_sp80090a.h>
#include <crypto/internal/cipher.h>
#include <linux/kernel.h>
#include <linux/jiffies.h>
#include <linux/string_choices.h>
+#include <linux/unaligned.h>
/***************************************************************
* Backend cipher definitions available to DRBG
***************************************************************/
@@ -599,11 +600,11 @@ static int drbg_hash_df(struct drbg_state *drbg,
unsigned char *tmp = drbg->scratchpad + drbg_statelen(drbg);
struct drbg_string data;
/* 10.4.1 step 3 */
input[0] = 1;
- drbg_cpu_to_be32((outlen * 8), &input[1]);
+ put_unaligned_be32(outlen * 8, &input[1]);
/* 10.4.1 step 4.1 -- concatenation of data for input into hash */
drbg_string_fill(&data, input, 5);
list_add(&data.list, entropylist);
diff --git a/include/crypto/internal/drbg.h b/include/crypto/internal/drbg.h
index 371e52dcee6c..b4e5ef0be602 100644
--- a/include/crypto/internal/drbg.h
+++ b/include/crypto/internal/drbg.h
@@ -7,28 +7,10 @@
*/
#ifndef _INTERNAL_DRBG_H
#define _INTERNAL_DRBG_H
-/*
- * Convert an integer into a byte representation of this integer.
- * The byte representation is big-endian
- *
- * @val value to be converted
- * @buf buffer holding the converted integer -- caller must ensure that
- * buffer size is at least 32 bit
- */
-static inline void drbg_cpu_to_be32(__u32 val, unsigned char *buf)
-{
- struct s {
- __be32 conv;
- };
- struct s *conversion = (struct s *)buf;
-
- conversion->conv = cpu_to_be32(val);
-}
-
/*
* Concatenation Helper and string operation helper
*
* SP800-90A requires the concatenation of different data. To avoid copying
* buffers around or allocate additional memory, the following data structure
--
2.53.0
^ permalink raw reply related
* [PATCH 01/38] crypto: drbg - Fix returning success on failure in CTR_DRBG
From: Eric Biggers @ 2026-04-20 6:33 UTC (permalink / raw)
To: linux-crypto, Herbert Xu
Cc: linux-kernel, Stephan Mueller, Jason A . Donenfeld, Eric Biggers,
stable
In-Reply-To: <20260420063422.324906-1-ebiggers@kernel.org>
drbg_ctr_generate() sometimes returns success when it fails, leaving the
output buffer uninitialized. Fix it.
Fixes: cde001e4c3c3 ("crypto: rng - RNGs must return 0 in success case")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
crypto/drbg.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/crypto/drbg.c b/crypto/drbg.c
index 9204e6edb426..e4eb78ed222b 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -375,11 +375,11 @@ static int drbg_ctr_generate(struct drbg_state *drbg,
/* 10.2.1.5.2 step 2 */
if (addtl && !list_empty(addtl)) {
ret = drbg_ctr_update(drbg, addtl, 2);
if (ret)
- return 0;
+ return ret;
}
/* 10.2.1.5.2 step 4.1 */
ret = drbg_kcapi_sym_ctr(drbg, NULL, 0, buf, len);
if (ret)
base-commit: c1f49dea2b8f335813d3b348fd39117fb8efb428
--
2.53.0
^ permalink raw reply related
* Re: [REGRESSION] Return change in 6.12.80+ with volatile mounting
From: Chenglong Tang @ 2026-04-20 6:31 UTC (permalink / raw)
To: Amir Goldstein; +Cc: Derek Taylor, stable, regressions, Kevin Berry, overlayfs
In-Reply-To: <CAOQ4uxhUn6oCBuVJqZu+FcMx8XeAQHZbXFAGon4Xeg2SPLJW_A@mail.gmail.com>
Hi, Amir,
Thanks for looking into this! To answer your questions:
1. Production vs. Test Suite Impact
The immediate failure we encountered is in containerd's integration
test suite (TestImageVolumeCheckVolatileOption). The test explicitly
reads /proc/mounts and expects the exact string "volatile".
In default production, containerd passes the legacy "volatile" string
to the mount syscall, which your patch correctly handles under the
hood. So the standard "happy path" is not broken in production.
2. The purpose of WithTempMount() / RemoveVolatileOption
Containerd regularly makes temporary overlay mounts (e.g., for
unpacking layers). Because overlayfs rejects reusing upper/work dirs
from a volatile mount, containerd uses RemoveVolatileOption to strip
the volatile flag before these temporary mounts.
Currently, containerd's RemoveVolatileOption does an exact string
match for "volatile". While it works for the default path, there is a
production edge case: if a user explicitly configures their container
runtime to use the new "fsync=volatile" option, older containerd
binaries will fail to strip it, and the temporary mounts will be
rejected by the kernel.
Conclusion
While containerd could theoretically patch their code to accept
strings.Contains() or fsync=volatile going forward, there are many
existing containerd binaries in the wild. Given that this patch breaks
containerd's CI tests and introduces an edge case for
RemoveVolatileOption, it might be safest to fix ovl_show_options in
the kernel to continue outputting the legacy "volatile" string to
strictly guarantee backwards compatibility with userspace.
Thanks,
Chenglong
On Sat, Apr 18, 2026 at 8:40 AM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Sat, Apr 18, 2026 at 1:33 AM Chenglong Tang <chenglongtang@google.com> wrote:
> >
> > CC Amir,
> >
> > For example, containerd 2.2.0 uses `volatile` instead of `fsync=volatile`:
> > https://github.com/containerd/containerd/blob/main/core/mount/temp.go#L91C1-L92C1
> >
> > On Fri, Apr 17, 2026 at 3:41 PM Derek Taylor <ddtaylor@google.com> wrote:
> > >
> > > This change seems to have so far affected at least containerd in an
> > > issue reported here
> > > https://github.com/containerd/containerd/issues/13250.
> > >
> > > In stable versions 6.12.80+, commit
> > > 6c0cfbe020c0fcd2a544fcd2931fbc366ee3cd12 with the specific change
> > > being:
> > > [*] The mount option "volatile" is an alias to "fsync=volatile".
> > > In this scenario, code relying on checking "volatile" will now fail
> > > due to the return being "fsync=volatile".
> > >
> > > #regzbot introduced:v6.12.80
>
> Hi Chenglong,
>
> Thanks for the report.
>
> Is this problem in production containerd or in a test suite?
> I did not understand the purpose of WithTempMount().
>
> Is it possible to fix this function to use string.Contains() instead of
> exact match to the "volatile" mount option?
>
> If needed I can fix the kernel to show the legacy "volatile" option,
> but I would like to first understand how bad the impact of this regression
> is on real production workloads.
>
> Thanks,
> Amir.
^ permalink raw reply
* Re: [PATCH v2] dmaengine: Fix refcount leak in channel register error path
From: Frank Li @ 2026-04-20 6:23 UTC (permalink / raw)
To: Guangshuo Li; +Cc: Vinod Koul, Dave Jiang, dmaengine, linux-kernel, stable
In-Reply-To: <20260413135857.2898676-1-lgs201920130244@gmail.com>
On Mon, Apr 13, 2026 at 09:58:57PM +0800, Guangshuo Li wrote:
> After device_register(), the lifetime of the embedded struct device is
> expected to be managed through the device core reference counting.
>
> In __dma_async_device_channel_register(), if device_register() fails,
> the error path frees chan->dev directly instead of releasing the device
> reference with put_device(). This bypasses the normal device lifetime
> rules and may leave the reference count of the embedded struct device
> unbalanced, resulting in a refcount leak.
>
> The issue was identified by a static analysis tool I developed and
> confirmed by manual review.
I think it is meanless, no one reproduce this. Provide tools link if open
source. Or you descript how problem happen.
> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index ca13cd39330b..6bb1212ae0e1 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
> @@ -1111,8 +1111,12 @@ static int __dma_async_device_channel_register(struct dma_device *device,
>
> err_out_ida:
> ida_free(&device->chan_ida, chan->chan_id);
> + put_device(&chan->dev->device);
> + chan->dev = NULL;
> + goto err_free_local;
avoid err path goto again
Frank
> err_free_dev:
> kfree(chan->dev);
> + chan->dev = NULL;
> err_free_local:
> free_percpu(chan->local);
> chan->local = NULL;
> --
> 2.43.0
>
^ permalink raw reply
* Re: [PATCH v2] drm/bridge: dw-hdmi-qp: Guard clear_audio_infoframe when PHY is down
From: Frank Zhang @ 2026-04-20 6:11 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: andrzej.hajda, neil.armstrong, rfoss, maarten.lankhorst, mripard,
tzimmermann, airlied, simona, detlev.casanova, cristian.ciocaltea,
Laurent.pinchart, jonas, jernej.skrabec, dri-devel, linux-kernel,
stable
In-Reply-To: <hdl63shkqubkvczlg7ryjah5psiqzrhu5llelzaetw7skbpujv@nyxgriryjxd5>
On 4/19/26 08:40, Dmitry Baryshkov wrote:
> On Sat, Apr 18, 2026 at 06:19:36PM +0800, Frank Zhang wrote:
>> The following panic was observed during system reboot:
>>
>> Kernel panic - not syncing: Asynchronous SError Interrupt
>> CPU: 7 UID: 1000 PID: 2637 Comm: pipewire ... 6.19.10-300.fc44.aarch64
>> Call trace:
>> ...
>> regmap_update_bits_base+0x5c/0x90
>> dw_hdmi_qp_bridge_clear_infoframe+0xb0/0x120 [dw_hdmi_qp]
>> drm_bridge_connector_clear_infoframe+0x28/0x48 [drm_display_helper]
>> ...
>> dw_hdmi_qp_audio_disable+0x24/0xb8 [dw_hdmi_qp]
>> drm_bridge_connector_audio_shutdown+0x30/0x60 [drm_display_helper]
>> drm_connector_hdmi_audio_shutdown+0x24/0x38 [drm_display_helper]
>> hdmi_codec_shutdown+0x60/0x90 [snd_soc_hdmi_codec]
>> ...
>> snd_pcm_release_substream.part.0+0x44/0xd8 [snd_pcm]
>> snd_pcm_release+0x60/0xe8 [snd_pcm]
>> ...
>>
>> The root cause is pipewire tries to close the HDMI audio device after
>> atomic_disable(), which sets tmds_char_rate to 0 and disable the PHY.
>>
>> In this case, dw_hdmi_qp_audio_disable() will call
>> drm_atomic_helper_connector_hdmi_clear_audio_infoframe() directly,
>> accessing registers without checking tmds_char_rate.
>>
>> Move drm_atomic_helper_connector_hdmi_clear_audio_infoframe() inside the
>> if (hdmi->tmds_char_rate) of dw_hdmi_qp_audio_disable().
>>
>> Fixes: fd0141d1a8a2 ("drm/bridge: synopsys: Add audio support for dw-hdmi-qp")
>> Signed-off-by: Frank Zhang <rmxpzlb@gmail.com>
>>
>> ---
>> Changes in v2:
>> - Move drm_atomic_helper_connector_hdmi_clear_audio_infoframe() inside
>> the if (hdmi->tmds_char_rate) of dw_hdmi_qp_audio_disable().
>> - Link to v1: https://lore.kernel.org/all/20260416093150.13853-1-rmxpzlb@gmail.com/
>>
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
>> index d649a1cf07f5..7760527484c8 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c
>> @@ -526,10 +526,10 @@ static void dw_hdmi_qp_audio_disable(struct drm_bridge *bridge,
>> {
>> struct dw_hdmi_qp *hdmi = dw_hdmi_qp_from_bridge(bridge);
>>
>> - drm_atomic_helper_connector_hdmi_clear_audio_infoframe(connector);
>> -
>> - if (hdmi->tmds_char_rate)
>> + if (hdmi->tmds_char_rate) {
>> + drm_atomic_helper_connector_hdmi_clear_audio_infoframe(connector);
>> dw_hdmi_qp_audio_disable_regs(hdmi);
>> + }
>
> Will audio and audio infoframe remain disabled after consequetive
> atomic_enable() call?
>
>> }
>>
>> static int dw_hdmi_qp_i2c_read(struct dw_hdmi_qp *hdmi,
>> --
>> 2.53.0
>>
>
Sorry, I missed clearing the audio infoframe when the PHY is down. The
next atomic_enable() will write the stale audio infoframe. My mistake.
To clear the stale audio infoframe, dw_hdmi_qp_audio_disable() can
handle it in the else branch directly, but this seems like a layering
violation for a bridge driver
I think the better approach is to add a 'reset_audio_infoframe'
interface in drm_hdmi_state_helper.c that does basically the same as
drm_atomic_helper_connector_hdmi_clear_audio_infoframe(), but only
clearing the software state without calling clear_infoframe(). It's also
a bit odd since it would only be used by dw-hdmi-qp.
I'd like to get the maintainers' opinion about adding such an interface.
Thanks,
Frank Zhang
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox