Re: [PATCH v2 -mm 4/6] ide: avoid DMA on the stack for REQ_TYPE_ATA_PC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 05 Jun 2008 11:10:48 -0500
James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote:

> On Mon, 2008-06-02 at 15:57 +0900, FUJITA Tomonori wrote:
> > Some REQ_TYPE_ATA_PC commands uses the stack buffers for DMA, which
> > leads to memory corruption on a non-coherent platform.
> > 
> > With regard to alignment and padding, ide-cd has the the dma safe
> > check for sg requests and REQ_TYPE_ATA_PC. This adds the stack buffer
> > check to that check.
> > 
> > Signed-off-by: FUJITA Tomonori <fujita.tomonori@xxxxxxxxxxxxx>
> > Cc: Borislav Petkov <petkovbb@xxxxxxxxx>
> > Cc: Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx>
> > ---
> >  drivers/ide/ide-cd.c |    5 +++++
> >  1 files changed, 5 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
> > index e3f085c..e12d602 100644
> > --- a/drivers/ide/ide-cd.c
> > +++ b/drivers/ide/ide-cd.c
> > @@ -1195,6 +1195,7 @@ static ide_startstop_t cdrom_do_block_pc(ide_drive_t *drive, struct request *rq)
> >  		struct request_queue *q = drive->queue;
> >  		unsigned int alignment;
> >  		unsigned long addr;
> > +		unsigned long stack_mask = ~(THREAD_SIZE - 1);
> >  
> >  		if (rq->bio)
> >  			addr = (unsigned long)bio_data(rq->bio);
> > @@ -1212,6 +1213,10 @@ static ide_startstop_t cdrom_do_block_pc(ide_drive_t *drive, struct request *rq)
> >  		alignment = queue_dma_alignment(q) | q->dma_pad_mask;
> >  		if (addr & alignment || rq->data_len & alignment)
> >  			info->dma = 0;
> > +
> > +		if (!((addr & stack_mask) ^
> > +		      ((unsigned long)current->stack & stack_mask)))
> 
> That can basically become
> 
> if ((addr & stack_mask)  == ((unsigned long)current->stack & stack_mask))
> 
> to be a bit clearer, can't it?

Ok, I'll do next time.


> I'm also not keen on the use of current->stack.  It looks like this
> commit:
> 
> commit f7e4217b007d1f73e7e3cf10ba4fea4a608c603f
> Author: Roman Zippel <zippel@xxxxxxxxxxxxxx>
> Date:   Wed May 9 02:35:17 2007 -0700
> 
>     rename thread_info to stack
> 
> Introduced a task_stack_page() accessor to get this instead, so perhaps
> we should use it (I've cc'd Roman and linux-arch for opinions).

Thanks. Seems that we should use it here (though current->stack look
like work now).

lib/debugobjects.c has the same code (that uses current->stack) to
detect an address on the stack.

Next time, I'll add a helper function to detect an address on the
stack, which is useful for debugobjects.c, blk-map.c and ide-cd.c.
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux