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  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:   Mon, 6 Nov 2017 07:24:52 -0800
From:   Tejun Heo <tj@...nel.org>
To:     Taras Kondratiuk <takondra@...co.com>
Cc:     linux-ide@...r.kernel.org, linux-kernel@...r.kernel.org,
        xe-linux-external@...co.com, Lin Ming <minggr@...il.com>
Subject: Re: Manual unbind of ATA devices causes use-after-free

Hello,

On Fri, Nov 03, 2017 at 09:32:16AM -0700, Taras Kondratiuk wrote:
> Quoting Tejun Heo (2017-11-03 06:19:37)
> > Hello,
> > 
> > On Wed, Nov 01, 2017 at 04:24:47PM -0700, Taras Kondratiuk wrote:
> > > Manual unbind/remove unconditionally invokes devres_release_all which
> > > calls ata_host_release() and frees ata_host/ata_port memory while it is
> > > still being referenced (e.g as a parent of SCSI host).
> > > 
> > > Is there a reason why ata_host is using derves which is not refcounted?
> > > Does it make sense to add recounting to ata_host?
> > 
> > Hmm... the removal path is supposed to drain everything synchronously.
> > What kind of controller is it?
> 
> It drains synchronously if scsi_host_put(ap->scsi_host) in
> ata_host_release() releases the last scsi_host reference. But when the issue
> happens there is one more reference to scsi_host because sg device is
> still open. The last reference will be dropped from sg_release.

I see.

> I forgot to mention that the disk may not be clearly unmounted when I'm
> unbinding it, but IMO it shouldn't cause use-after-free in the kernel.

Oh, it shouldn't.  It's a bug.

> Also even if sg_release() is called before ata_host_release() there is
> still no guarantee that the last reference will be dropped, because
> sg_release() schedules sg_remove_sfp_usercontext() to do actual release
> and the work may not be completed in time.

Hmmm, we didn't use to put in ata device structs in the kobject tree,
so this wasn't an issue.  This was changed by 9a6d6a2ddabb ("ata: make
ata port as parent device of scsi host") while adding the transport
support.  While doing that, we didn't change the release path to match
it, so the failure.

cc'ing Lin.  Lin, can you take a look at this?

Thanks.

-- 
tejun

Powered by blists - more mailing lists