linux-kernel - Re: [syzbot] [overlayfs?] WARNING in ovl_encode_real

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <tencent_73EE0DCC923DDDAB5DD8995C4F958DE92507@qq.com>
Date: Wed,  6 Nov 2024 18:18:00 +0800
From: Edward Adam Davis <eadavis@...com>
To: amir73il@...il.com
Cc: eadavis@...com,
	linux-kernel@...r.kernel.org,
	linux-unionfs@...r.kernel.org,
	miklos@...redi.hu,
	syzbot+ec07f6f5ce62b858579f@...kaller.appspotmail.com,
	syzkaller-bugs@...glegroups.com
Subject: Re: [syzbot] [overlayfs?] WARNING in ovl_encode_real_fh

On Wed, 6 Nov 2024 09:20:24 +0100, Amir Goldstein <amir73il@...il.com> wrote:
> On Wed, Nov 6, 2024 at 3:43 AM Edward Adam Davis <eadavis@...com> wrote:
> >
> > On Mon, 4 Nov 2024 20:30:41 +0100, Amir Goldstein <amir73il@...il.com> wrote:
> > > > When the memory is insufficient, the allocation of fh fails, which causes
> > > > the failure to obtain the dentry fid, and finally causes the dentry encoding
> > > > to fail.
> > > > Retry is used to avoid the failure of fh allocation caused by temporary
> > > > insufficient memory.
> > > >
> > > > #syz test
> > > >
> > > > diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> > > > index 2ed6ad641a20..1e027a3cf084 100644
> > > > --- a/fs/overlayfs/copy_up.c
> > > > +++ b/fs/overlayfs/copy_up.c
> > > > @@ -423,15 +423,22 @@ struct ovl_fh *ovl_encode_real_fh(struct ovl_fs *ofs, struct dentry *real,
> > > >         int fh_type, dwords;
> > > >         int buflen = MAX_HANDLE_SZ;
> > > >         uuid_t *uuid = &real->d_sb->s_uuid;
> > > > -       int err;
> > > > +       int err, rtt = 0;
> > > >
> > > >         /* Make sure the real fid stays 32bit aligned */
> > > >         BUILD_BUG_ON(OVL_FH_FID_OFFSET % 4);
> > > >         BUILD_BUG_ON(MAX_HANDLE_SZ + OVL_FH_FID_OFFSET > 255);
> > > >
> > > > +retry:
> > > >         fh = kzalloc(buflen + OVL_FH_FID_OFFSET, GFP_KERNEL);
> > > > -       if (!fh)
> > > > +       if (!fh) {
> > > > +               if (!rtt) {
> > > > +                       cond_resched();
> > > > +                       rtt++;
> > > > +                       goto retry;
> > > > +               }
> > > >                 return ERR_PTR(-ENOMEM);
> > > > +       }
> > > >
> > > >         /*
> > > >          * We encode a non-connectable file handle for non-dir, because we
> > > >
> > >
> > > This endless loop is out of the question and anyway, syzbot reported
> > > a WARN_ON in line 448:
> > >             WARN_ON(fh_type == FILEID_INVALID))
> > >
> > > How does that have to do with memory allocation failure?
> > > What am I missing?
> > Look following log, it in https://syzkaller.appspot.com/text?tag=CrashLog&x=178bf640580000:
> > [   64.050342][ T5103] FAULT_INJECTION: forcing a failure.
> > [   64.050342][ T5103] name failslab, interval 1, probability 0, space 0, times 0
> > [   64.055933][ T5103] CPU: 0 UID: 0 PID: 5103 Comm: syz-executor195 Not tainted 6.12.0-rc4-syzkaller-00047-gc2ee9f594da8 #0
> > [   64.060023][ T5103] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2~bpo12+1 04/01/2014
> > [   64.063941][ T5103] Call Trace:
> > [   64.065199][ T5103]  <TASK>
> > [   64.066296][ T5103]  dump_stack_lvl+0x241/0x360
> > [   64.068028][ T5103]  ? __pfx_dump_stack_lvl+0x10/0x10
> > [   64.069939][ T5103]  ? __pfx__printk+0x10/0x10
> > [   64.071667][ T5103]  ? __kmalloc_cache_noprof+0x44/0x2c0
> > [   64.073756][ T5103]  ? __pfx___might_resched+0x10/0x10
> > [   64.075720][ T5103]  should_fail_ex+0x3b0/0x4e0
> > [   64.077525][ T5103]  should_failslab+0xac/0x100
> > [   64.079341][ T5103]  ? ovl_encode_real_fh+0xdf/0x410
> > [   64.081295][ T5103]  __kmalloc_cache_noprof+0x6c/0x2c0
> > [   64.083282][ T5103]  ? dput+0x37/0x2b0
> > [   64.084758][ T5103]  ovl_encode_real_fh+0xdf/0x410
> > [   64.086578][ T5103]  ? __pfx_ovl_encode_real_fh+0x10/0x10
> > [   64.088687][ T5103]  ? _raw_spin_unlock+0x28/0x50
> > [   64.090550][ T5103]  ovl_encode_fh+0x388/0xc20
> > [   64.092281][ T5103]  exportfs_encode_fh+0x1bd/0x3e0
> > [   64.094122][ T5103]  ovl_encode_real_fh+0x129/0x410
> > [   64.095883][ T5103]  ? __pfx_ovl_encode_real_fh+0x10/0x10
> > [   64.097852][ T5103]  ? bpf_lsm_capable+0x9/0x10
> > [   64.099620][ T5103]  ? capable+0x89/0xe0
> > [   64.101064][ T5103]  ovl_copy_up_flags+0x1068/0x46f0
> 
> I see. it is nested overlayfs, so a memory allocation failure in the lower
> overlayfs, causes ovl_encode_fh() to return FILEID_INVALID.
> 
> > >
> > > Probably this WARN_ON as well as the one in line 446 should be
> > > relaxed because it is perfectly possible for fs to return negative or
> > > FILEID_INVALID for encoding a file handle even if fs supports encoding
> > > file handles.
> > >
> 
> As I wrote, the correct fix is to relax the WARN_ON from
> fh_type == FILEID_INVALID and fh_type < 0 conditions because
> those are valid return values from filesystems.
Oh, You mean is following diff?
diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 2ed6ad641a20..32890cc0dd4a 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -443,9 +443,7 @@ struct ovl_fh *ovl_encode_real_fh(struct ovl_fs *ofs, struct dentry *real,
 	buflen = (dwords << 2);
 
 	err = -EIO;
-	if (WARN_ON(fh_type < 0) ||
-	    WARN_ON(buflen > MAX_HANDLE_SZ) ||
-	    WARN_ON(fh_type == FILEID_INVALID))
+	if (WARN_ON(buflen > MAX_HANDLE_SZ))
 		goto out_err;
 
 	fh->fb.version = OVL_FH_VERSION;

BR,
Edward