> On Fri, Nov 24, 2023 at 5:26 PM Jann Horn <jannh@xxxxxxxxxx> wrote: >> >> On Fri, Nov 24, 2023 at 4:11 PM Jann Horn <jannh@xxxxxxxxxx> wrote: >> > >> > On Wed, Sep 27, 2023 at 5:10 PM syzbot >> > <syzbot+477d8d8901756d1cbba1@xxxxxxxxxxxxxxxxxxxxxxxxx> wrote: >> > > syzbot has tested the proposed patch and the reproducer did not trigger any issue: >> > > >> > > Reported-and-tested-by: syzbot+477d8d8901756d1cbba1@xxxxxxxxxxxxxxxxxxxxxxxxx >> > > >> > > Tested on: >> > > >> > > commit: 8e9b46c4 ovl: do not encode lower fh with upper sb_wri.. >> > > git tree: https://github.com/amir73il/linux.git ovl_want_write >> > > console output: https://syzkaller.appspot.com/x/log.txt?x=10d10ffa680000 >> > > kernel config: https://syzkaller.appspot.com/x/.config?x=bb54ecdfa197f132 >> > > dashboard link: https://syzkaller.appspot.com/bug?extid=477d8d8901756d1cbba1 >> > > compiler: gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40 >> > >> > It looks like the fix was submitted without the Reported-by tag, so >> > syzkaller doesn't recognize that the fix has landed... I'll tell >> > syzkaller now which commit the fix is supposed to be in, please >> > correct me if this is wrong: >> > >> > #syz fix: ovl: do not encode lower fh with upper sb_writers held >> >> (Ah, and just for the record: I hadn't realized when writing this that >> the fix was actually in a newer version of the same patch... "git > > That is correct. > I am very thankful for syzbot with helping me catch bugs during development > and I would gladly attribute the bot and its owners, but I don't that > Reported-and-tested-by is an adequate tag for a bug that never existed as > far as git history. > > Even Tested-by: syzbot could be misleading to stable kernel bots > that may conclude that the patch is a fix that needs to apply to stable. > > I am open to suggestions. > > Also maybe > > #syz correction: unknown command "correction:" > > To tell syzbot we are not fixing a bug in upstream, but in a previous > version of a patch that it had tested. > >> range-diff 44ef23e481b02df2f17599a24f81cf0045dc5256~1..44ef23e481b02df2f17599a24f81cf0045dc5256 >> 5b02bfc1e7e3811c5bf7f0fa626a0694d0dbbd77~1..5b02bfc1e7e3811c5bf7f0fa626a0694d0dbbd77" >> shows an added "ovl_get_index_name", I guess that's the fix?) > > No, that added ovl_get_index_name() seems like a fluke of the range-diff tool. > All the revisions of this patch always had this same minor change in this line: > > - err = ovl_get_index_name(ofs, c->lowerpath.dentry, > &c->destname); > + err = ovl_get_index_name(ofs, origin, &c->destname); > > The fix is obviously in the other part of the range-diff. > > Thanks, > Amir. > > if (err) > - return err; > -+ goto out; > ++ goto out_free_fh; > } else if (WARN_ON(!c->parent)) { > /* Disconnected dentry must be copied up to index dir */ > - return -EIO; > + err = -EIO; > -+ goto out; > ++ goto out_free_fh; > } else { > /* > * Mark parent "impure" because it may now contain non-pure > @@ fs/overlayfs/copy_up.c: static int ovl_do_copy_up(struct > ovl_copy_up_ctx *c) > ovl_end_write(c->dentry); > if (err) > - return err; > -+ goto out; > ++ goto out_free_fh; > } > > /* Should we copyup with O_TMPFILE or with workdir? */ > @@ fs/overlayfs/copy_up.c: static int ovl_do_copy_up(struct > ovl_copy_up_ctx *c) > out: > if (to_index) > kfree(c->destname.name); > ++out_free_fh: > + kfree(fh); > return err; > }
