MIME-Version: 1.0
References: <163906878733.143852.5604115678965006622.stgit@warthog.procyon.org.uk>
 <CALF+zOnA2U6LjDTE8m2REDTMmFVnWkcBkn0ZJQRGULPUjeQW4Q@mail.gmail.com>
 <CALF+zOnmJ0=j8pEMikpxYgLrS10gVZiXfCjBhDz9Je0Qip7wnw@mail.gmail.com> <599331.1639410068@warthog.procyon.org.uk>
In-Reply-To: <599331.1639410068@warthog.procyon.org.uk>
From:   David Wysochanski <dwysocha@redhat.com>
Date:   Mon, 13 Dec 2021 16:05:25 -0500
Message-ID: <CALF+zO=_DeOHLARqFqZ2qL04PcwBGOxTXf5UUdBNQj29yOSYvw@mail.gmail.com>
Subject: Re: [PATCH] fscache: Need to go round again after processing
 LRU_DISCARDING state
To:     David Howells <dhowells@redhat.com>
Cc:     linux-cachefs <linux-cachefs@redhat.com>,
        linux-nfs <linux-nfs@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Dec 13, 2021 at 10:41 AM David Howells <dhowells@redhat.com> wrote:
>
> David Wysochanski <dwysocha@redhat.com> wrote:
>
> > > [  432.921382] BUG: KASAN: use-after-free in
> > > fscache_unhash_cookie+0x9e/0x160 [fscache]^M
>
> I think the patch below is the way to fix this.
>
> David
> ---
> fscache: Need to go round again after processing LRU_DISCARDING state
>
> There's a race between the LRU discard and relinquishment actions.  In the
> state machine, fscache_cookie_state_machine(), the ACTIVE state transits to
> the LRU_DISCARD state in preference to transiting to the RELINQUISHING or
> WITHDRAWING states.
>
> This should be fine, but the LRU_DISCARDING state just breaks out the
> bottom of the function without going round again after transiting to the
> QUIESCENT state.
>
> However, if both LRU discard and relinquishment happen *before* the SM
> runs, one of the queue events will get discarded, along with the ref that
> would be associated with it.  The last ref is then discarded and the cookie
> is removed without completing the relinquishment process - leaving the
> cookie hashed.
>
> The fix is to make sure that the SM always goes back around after changing
> the state.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
>
> diff --git a/fs/fscache/cookie.c b/fs/fscache/cookie.c
> index d7e825d636e2..8d0769a5ee2b 100644
> --- a/fs/fscache/cookie.c
> +++ b/fs/fscache/cookie.c
> @@ -755,7 +755,7 @@ static void fscache_cookie_state_machine(struct fscache_cookie *cookie)
>                 set_bit(FSCACHE_COOKIE_NO_DATA_TO_READ, &cookie->flags);
>                 __fscache_set_cookie_state(cookie, FSCACHE_COOKIE_STATE_QUIESCENT);
>                 wake = true;
> -               break;
> +               goto again_locked;
>
>         case FSCACHE_COOKIE_STATE_DROPPED:
>                 break;
>

Agree and verified with xfstests generic full runs twice with NFSv3.
Prior to this patch with NFSv3 xfstest I'd regularly see the crash:
BUG: KASAN: use-after-free in __fscache_acquire_cookie+0x437
https://marc.info/?l=v9fs-developer&m=163916153103008&w=2
https://marc.info/?l=linux-nfs&m=163917893813589&w=2

Tested-by: Dave Wysochanski <dwysocha@redhat.com>