Greg Kroah-Hartman wrote on Sun, Jan 12, 2025 at 11:03:42AM +0100:
> On Fri, Jan 10, 2025 at 04:58:41PM +0900, Dominique Martinet wrote:
> > I've picked the "do not keep dangling zcomp pointer" patch from the
> > linux-rc tree at the time, so kept Sasha's SOB and added mine on top
> > -- please let me know if it wasn't appropriate.
>
> It's tricky to know, I dropped it and took what was in Linus's tree as
> Sasha didn't actually review this one.
Thanks for saying this; I hadn't actually checked the stable backport
(enough) either so I had another look, and the 6d2453c3dbc5
("drivers/block/zram/zram_drv.c: do not keep dangling zcomp pointer
after zram reset") backport by itself is wrong even if it did
cherry-pick cleanly from master and a quick test appeared to work.
The commit messages says "We do all reset operations under write lock"
but that isn't true without also backporting 6f1637795f28 ("zram: fix
race between zram_reset_device() and disksize_store()"), so with the
current backport we've traded leaking zram->comp behind with a data race
on disksize and comp.
With that extra commit as well, I think we're sane enough, but I'm not
familiar with the zram code so I might have missed another prerequisite.
With that and the previous problems, and given that manipulating
zram devices is a privileged operation (so we're not looking at a must
fix vulnerability), I'm actually rather inclined to just drop all the
zram patches and not backport these to 5.15/5.10 unless someone actually
reports problems around zram reset (or perhaps keep 5.15 but skip 5.10
as you see fit)
(
I'm not opposed to Kairui or someone else actually do these backport,
but I'm not confident it's worth the effort and think we're trading a
known problem (current behaviour) with potential unknown ones if
we're just cherry-picking an arbitrary subset of patches.
If someone wants to take over, the commits I had identified (from
Sasha's initial backport and this mail) for 5.10 were as follow:
3b4f85d02a4b ("loop: let set_capacity_revalidate_and_notify update the bdev size")
5dd55749b79c ("nvme: let set_capacity_revalidate_and_notify update the bdev size")
b200e38c493b ("sd: update the bdev size in sd_revalidate_disk")
449f4ec9892e ("block: remove the update_bdev parameter to set_capacity_revalidate_and_notify")
6e017a3931d7 ("zram: use set_capacity_and_notify")
6f1637795f28 ("zram: fix race between zram_reset_device() and disksize_store()")
6d2453c3dbc5 ("drivers/block/zram/zram_drv.c: do not keep dangling zcomp pointer after zram reset")
677294e4da96 ("zram: check comp is non-NULL before calling comp_destroy")
74363ec674cb ("zram: fix uninitialized ZRAM not releasing backing device")
)
Thank you Greg for the follow-ups, and thank you Kairui for the
suggestions during my earlier bug report.
--
Dominique
