[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAGXu5jJK3JuwLBGLbS1i6hBdyg_mkbVibbu6irfsnTOE1QhtfA@mail.gmail.com>
Date: Tue, 17 Apr 2018 09:30:59 -0700
From: Kees Cook <keescook@...omium.org>
To: James Bottomley <jejb@...ux.vnet.ibm.com>
Cc: Oleksandr Natalenko <oleksandr@...alenko.name>,
Jens Axboe <axboe@...nel.dk>,
Bart Van Assche <bart.vanassche@....com>,
Paolo Valente <paolo.valente@...aro.org>,
David Windsor <dave@...lcore.net>,
"Martin K. Petersen" <martin.petersen@...cle.com>,
linux-scsi@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>,
Christoph Hellwig <hch@....de>,
Hannes Reinecke <hare@...e.com>,
Johannes Thumshirn <jthumshirn@...e.de>,
linux-block@...r.kernel.org
Subject: Re: usercopy whitelist woe in scsi_sense_cache
On Tue, Apr 17, 2018 at 3:02 AM, James Bottomley
<jejb@...ux.vnet.ibm.com> wrote:
> On Mon, 2018-04-16 at 20:12 -0700, Kees Cook wrote:
>> I still haven't figured this out, though... any have a moment to look
>> at this?
>
> Just to let you know you're not alone ... but I can't make any sense of
> this either. The bfdq is the elevator_data, which is initialised when
> the scheduler is attached, so it shouldn't change. Is it possible to
> set a data break point on elevator_data after it's initialised and see
> if it got changed by something?
Yeah, it seems like some pointer chain is getting overwritten outside
of a lock or rcu or ?. I don't know this code well enough to guess at
where to check, though. What I find so strange is that the structure
offsets are different between bfpd's rq_in_driver field and
scsi_request's sense field, so even THAT doesn't look to be clear-cut
either:
struct bfq_data {
struct request_queue * queue; /* 0 8 */
struct list_head dispatch; /* 8 16 */
struct bfq_group * root_group; /* 24 8 */
struct rb_root queue_weights_tree; /* 32 8 */
struct rb_root group_weights_tree; /* 40 8 */
int busy_queues; /* 48 4 */
int wr_busy_queues; /* 52 4 */
int queued; /* 56 4 */
int rq_in_driver; /* 60 4 */
...
struct scsi_request {
unsigned char __cmd[16]; /* 0 16 */
unsigned char * cmd; /* 16 8 */
short unsigned int cmd_len; /* 24 2 */
/* XXX 2 bytes hole, try to pack */
int result; /* 28 4 */
unsigned int sense_len; /* 32 4 */
unsigned int resid_len; /* 36 4 */
int retries; /* 40 4 */
/* XXX 4 bytes hole, try to pack */
void * sense; /* 48 8 */
...
This is _so_ weird. :P
-Kees
--
Kees Cook
Pixel Security
Powered by blists - more mailing lists