Re: [syzbot] [overlayfs?] KASAN: invalid-free in ovl_copy_up_one

[Jann Horn]

On Sat, Nov 25, 2023 at 10:21 AM Amir Goldstein <amir73il@xxxxxxxxx> wrote:
>
> 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:
>
> To tell syzbot we are not fixing a bug in upstream, but in a previous
> version of a patch that it had tested.

Sorry, I think I got that wrong; the syzbot manual
(https://github.com/google/syzkaller/blob/master/docs/syzbot.md#rebuilt-treesamended-patches)
instead suggests:

"First, adding Reported-by tags to amended commits may be misleading.
A Reported-by tag suggests that the commit fixes a bug in some
previous commit, but here it's not the case (it only fixes a bug in a
previous version of itself which is not in the tree). In such case
it's recommended to include a Tested-by or a Reviewed-by tag with the
hash instead. Technically, syzbot accepts any tag, so With-inputs-from
would work too."

> > 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:

Ah, bleh, of course. I haven't used range-diff in a while and misread
the output...