linux-kernel - Re: refcount_t: underflow; use-after-free with CIFS umount after scsi-misc commit ef2cc88e2a205b8a11a19e78db63a70d3728cdf5

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]

Date:   Sun, 8 Dec 2019 21:18:51 -0600
From:   Steve French <smfrench@...il.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Arthur Marsh <arthur.marsh@...ernode.on.net>,
        SCSI development list <linux-scsi@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        CIFS <linux-cifs@...r.kernel.org>,
        "James E.J. Bottomley" <jejb@...ux.ibm.com>,
        ronnie sahlberg <ronniesahlberg@...il.com>
Subject: Re: refcount_t: underflow; use-after-free with CIFS umount after
 scsi-misc commit ef2cc88e2a205b8a11a19e78db63a70d3728cdf5

On Sun, Dec 8, 2019 at 8:23 PM Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> On Sun, Dec 8, 2019 at 5:49 PM Arthur Marsh
> <arthur.marsh@...ernode.on.net> wrote:
> >
> > This still happens with 5.5.0-rc1:
>
> Does it happen 100% of the time?

I can reproduce it (although it was a little more difficult since WiFi doesn't
work on RC1 on some of my hardware - due to the 802.11 driver regression oops.
I was able to reproduce it to Samba localhost).

> Your bisection result looks pretty nonsensical - not that it's
> impossible (anything is possible), but it really doesn't look very
> likely. Which makes me think maybe it's slightly timing-sensitive or
> something?

The bisection result is implausible.  I just did some experiments and
it looks far more likely is that it is related to commit
72e73c78c446e ("cifs: close the shared root handle on tree disconnect")
so added Ronnie to the cc.  That patch added a call (at unmount time)
to close_shroot.
The idea of that patch made sense - although tree disconnect (and then
logoff of the session)
will indirectly free any open handles on the server for that session,
it is a little
cleaner to close the cached root SMB3 file handle explicitly.

void close_shroot(struct cached_fid *cfid)
{
        mutex_lock(&cfid->fid_mutex);
        kref_put(&cfid->refcount, smb2_close_cached_fid);
        mutex_unlock(&cfid->fid_mutex);
}

Taking out the one line change in the patch from last week that calls
close_shroot from
umount (SMB2_tdis, ie tree_disconnect) I don't see the problem so far
more likely
that it is related to that commit.   The problem seems to be related
to servers which
don't support directory leases.  Will spin up a patch to fix this if
Ronnie hasn't already fixed it