Re: [PATCH v2] ovl: keep err zero after successful ovl_cache_get()
Nirmoy Das <[email protected]> Fri, 15 May 2026 13:15:41 +0200
| Newsgroups | org.kernel.vger.linux-unionfs,org.kernel.vger.linux-kernel,org.kernel.vger.stable |
|---|---|
| Message-ID | <[email protected]> |
Hi Amir, On 14.05.26 22:19, Amir Goldstein wrote: > On Thu, May 14, 2026 at 5:26 PM Amir Goldstein <[email protected]> wrote: >> On Thu, May 14, 2026 at 4:43 PM Nirmoy Das <[email protected]> wrote: >>> ovl_iterate_merged() stores PTR_ERR(cache) in err before checking >>> IS_ERR(cache). On success err holds the truncated cache pointer and >>> can be returned as a bogus non-zero error. >>> >>> The syzbot reproducer reaches this through overlay-on-overlay readdir: >>> >>> getdents64 >>> iterate_dir(outer overlay file) >>> ovl_iterate_merged() >>> ovl_cache_get() >>> ovl_dir_read_merged() >>> ovl_dir_read() >>> iterate_dir(inner overlay file) >>> ovl_iterate_merged() >>> >>> Only compute PTR_ERR(cache) on the error path. >>> >>> Fixes: d25e4b739f83 ("ovl: refactor ovl_iterate() and port to cred guard") >>> Reported-by: [email protected] >>> Closes: https://syzkaller.appspot.com/bug?extid=a16fb0cce329a320661c >>> Cc: [email protected] >>> Signed-off-by: Nirmoy Das <[email protected]> >>> --- >>> v2: >>> - Drop the now-redundant 'int err = 0' initializer and the trailing >>> 'return err' in ovl_iterate_merged(); err is only used inside the >>> loop's update-check, so the function can just return 0 on success. >>> (Amir Goldstein) >>> - Link to v1: >>> https://lore.kernel.org/all/[email protected]/ >>> >> I queue this up and will work on fortifying patches. > Nirmoy, > > I pushed fortify patches to ovl-fixes on my github [1]. > > Can you verify that the assertions trigger if you revert your fix > and run the reproducer? > > I imagine they would trigger much more frequently than the KASAN > warnings do. Yes, the assertion triggers with your ovl-fixes branch after reverting my fix. 9541f25af774 Revert "ovl: keep err zero after successful ovl_cache_get()" 1c067d912e47 ovl: add assertions in dir cache code 98e3a2d258e9 ovl: fix race between copy-up and open of a directory 4f80bb375112 ovl: keep err zero after successful ovl_cache_get() 18de6460b6bd ovl: opt-in for fortified ERR_PTR() 690bd87e1fef err_ptr.h: introduce ERR_PTR_SAFE() 7fd2df204f34 Linux 7.1-rc2 Running the syz reproducer with panic_on_warn=1 triggered: [ 55.404636] ------------[ cut here ]------------ [ 55.404646] WARNING: fs/overlayfs/readdir.c:511 at ovl_iterate+0x4c0/0x5bc, CPU#2: syz-ovl-iterate/14575 [ 55.406875] CPU: 2 UID: 0 PID: 14575 Comm: syz-ovl-iterate Not tainted 7.1.0-rc2-g9541f25af774 #1 PREEMPT [ 55.408328] pc : ovl_iterate+0x4c0/0x5bc [ 55.408632] lr : ovl_iterate+0x4b4/0x5bc [ 55.413504] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffffffffc152db40 [ 55.414036] Call trace: [ 55.414209] ovl_iterate+0x4c0/0x5bc (P) [ 55.414503] wrap_directory_iterator+0x60/0x90 [ 55.414809] shared_ovl_iterate+0x18/0x24 [ 55.415125] iterate_dir+0x10c/0x3a4 [ 55.415365] __arm64_sys_getdents64+0xe0/0x1e4 [ 55.417312] Kernel panic - not syncing: kernel: panic_on_warn set ... Regards, Nirmoy > > Thanks, > Amir. > > [1] https://github.com/amir73il/linux/commits/ovl-fixes/