All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Thumshirn <jthumshirn@suse.de>
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Dmitry Vyukov <dvyukov@google.com>,
	syzkaller <syzkaller@googlegroups.com>,
	Doug Gilbert <dgilbert@interlog.com>,
	jejb@linux.vnet.ibm.com,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	linux-scsi <linux-scsi@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	axboe@kernel.dk, linux-block@vger.kernel.org,
	David Rientjes <rientjes@google.com>,
	Hannes Reinecke <hare@suse.de>,
	mhocko@suse.cz
Subject: Re: scsi: use-after-free in bio_copy_from_iter
Date: Tue, 6 Dec 2016 10:32:57 +0100	[thread overview]
Message-ID: <20161206093257.GC4818@linux-x5ow.site> (raw)
In-Reply-To: <20161205190339.GK1555@ZenIV.linux.org.uk>

On Mon, Dec 05, 2016 at 07:03:39PM +0000, Al Viro wrote:
> On Mon, Dec 05, 2016 at 04:17:53PM +0100, Johannes Thumshirn wrote:
> > 633         hp = &srp->header;
> > [...]
> > 646                 hp->dxferp = (char __user *)buf + cmd_size;
> 
> > So the memory for hp->dxferp comes from:
> > 633         hp = &srp->header;
> 
> ????
> 
> > >From my debug instrumentation I see that the dxferp ends up in the
> > iovec_iter's kvec->iov_base and the faulting address is always dxferp + n *
> > 4k with n in [1, 16] (and we're copying 16 4k pages from the iovec into the
> > bio).
> 
> _Address_ of hp->dxferp comes from that assignment; the value is 'buf'
> argument of sg_write() + small offset.  In this case, it should point
> inside a pipe buffer, which is, indeed, at a kernel address.  Who'd
> allocated srp is irrelevant.

Yes I realized that as well when I had enough distance between me and the
code...

> 
> And if you end up dereferencing more than one page worth there, you do have
> a problem - pipe buffers are not going to be that large.  Could you slap
> 	WARN_ON((size_t)input_size > count);
> right after the calculation of input_size in sg_write() and see if it triggers
> on your reproducer?

I did and it didn't trigger. What triggers is (as expected) a
	WARN_ON((size_t)mxsize > count);
We have count at 80 and mxsize (which ends in hp->dxfer_len) at 65499. But the
65499 bytes are the len of the data we're suppost to be copying in via the
iov. I'm still rather confused what's happening here, sorry.

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N�rnberg
GF: Felix Imend�rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N�rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

WARNING: multiple messages have this Message-ID (diff)
From: Johannes Thumshirn <jthumshirn@suse.de>
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Dmitry Vyukov <dvyukov@google.com>,
	syzkaller <syzkaller@googlegroups.com>,
	Doug Gilbert <dgilbert@interlog.com>,
	jejb@linux.vnet.ibm.com,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	linux-scsi <linux-scsi@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	axboe@kernel.dk, linux-block@vger.kernel.org,
	David Rientjes <rientjes@google.com>,
	Hannes Reinecke <hare@suse.de>,
	mhocko@suse.cz
Subject: Re: scsi: use-after-free in bio_copy_from_iter
Date: Tue, 6 Dec 2016 10:32:57 +0100	[thread overview]
Message-ID: <20161206093257.GC4818@linux-x5ow.site> (raw)
In-Reply-To: <20161205190339.GK1555@ZenIV.linux.org.uk>

On Mon, Dec 05, 2016 at 07:03:39PM +0000, Al Viro wrote:
> On Mon, Dec 05, 2016 at 04:17:53PM +0100, Johannes Thumshirn wrote:
> > 633         hp = &srp->header;
> > [...]
> > 646                 hp->dxferp = (char __user *)buf + cmd_size;
> 
> > So the memory for hp->dxferp comes from:
> > 633         hp = &srp->header;
> 
> ????
> 
> > >From my debug instrumentation I see that the dxferp ends up in the
> > iovec_iter's kvec->iov_base and the faulting address is always dxferp + n *
> > 4k with n in [1, 16] (and we're copying 16 4k pages from the iovec into the
> > bio).
> 
> _Address_ of hp->dxferp comes from that assignment; the value is 'buf'
> argument of sg_write() + small offset.  In this case, it should point
> inside a pipe buffer, which is, indeed, at a kernel address.  Who'd
> allocated srp is irrelevant.

Yes I realized that as well when I had enough distance between me and the
code...

> 
> And if you end up dereferencing more than one page worth there, you do have
> a problem - pipe buffers are not going to be that large.  Could you slap
> 	WARN_ON((size_t)input_size > count);
> right after the calculation of input_size in sg_write() and see if it triggers
> on your reproducer?

I did and it didn't trigger. What triggers is (as expected) a
	WARN_ON((size_t)mxsize > count);
We have count at 80 and mxsize (which ends in hp->dxfer_len) at 65499. But the
65499 bytes are the len of the data we're suppost to be copying in via the
iov. I'm still rather confused what's happening here, sorry.

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

  reply	other threads:[~2016-12-06  9:32 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-25 19:08 scsi: use-after-free in bio_copy_from_iter Dmitry Vyukov
2016-12-02 16:50 ` Dmitry Vyukov
2016-12-03 10:38   ` Johannes Thumshirn
2016-12-03 10:38     ` Johannes Thumshirn
2016-12-03 15:22     ` Dmitry Vyukov
2016-12-03 18:19       ` Johannes Thumshirn
2016-12-03 18:19         ` Johannes Thumshirn
2016-12-05 14:31         ` Dmitry Vyukov
2016-12-05 15:17           ` Johannes Thumshirn
2016-12-05 15:17             ` Johannes Thumshirn
2016-12-05 19:03             ` Al Viro
2016-12-06  9:32               ` Johannes Thumshirn [this message]
2016-12-06  9:32                 ` Johannes Thumshirn
2016-12-06  9:43                 ` Dmitry Vyukov
2016-12-06 15:38                   ` Johannes Thumshirn
2016-12-06 15:38                     ` Johannes Thumshirn
2016-12-06 15:46                     ` Dmitry Vyukov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161206093257.GC4818@linux-x5ow.site \
    --to=jthumshirn@suse.de \
    --cc=axboe@kernel.dk \
    --cc=dgilbert@interlog.com \
    --cc=dvyukov@google.com \
    --cc=hare@suse.de \
    --cc=jejb@linux.vnet.ibm.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=mhocko@suse.cz \
    --cc=rientjes@google.com \
    --cc=syzkaller@googlegroups.com \
    --cc=viro@ZenIV.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.