linux-kernel - Re: [PATCH 04/40] ide-scsi: fix Interrupt Reason checking in idescsi_pc

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:	Tue, 27 May 2008 20:30:48 +0200
From:	Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
To:	petkovbb@...il.com
Cc:	linux-ide@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 04/40] ide-scsi: fix Interrupt Reason checking in idescsi_pc_intr()

On Monday 19 May 2008, Borislav Petkov wrote:
> On Sun, May 18, 2008 at 08:55:03PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > Set PC_FLAG_WRITING pc flag in idescsi_queue() (if needed)
> > and then fix Interrupt Reason checking in idescsi_pc_intr().
> > 
> > Cc: Borislav Petkov <petkovbb@...il.com>
> > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@...il.com>
> > ---
> >  drivers/scsi/ide-scsi.c |   17 +++++++++++------
> >  1 file changed, 11 insertions(+), 6 deletions(-)
> > 
> > Index: b/drivers/scsi/ide-scsi.c
> > ===================================================================
> > --- a/drivers/scsi/ide-scsi.c
> > +++ b/drivers/scsi/ide-scsi.c
> > @@ -427,7 +427,15 @@ static ide_startstop_t idescsi_pc_intr (
> >  		printk(KERN_ERR "ide-scsi: CoD != 0 in idescsi_pc_intr\n");
> >  		return ide_do_reset (drive);
> >  	}
> > -	if (ireason & IO) {
> > +	if (((ireason & IO) == IO) == !!(pc->flags & PC_FLAG_WRITING)) {
> > +		/* Hopefully, we will never get here */
> > +		printk(KERN_ERR "%s: We wanted to %s, but the device wants us "
> > +				"to %s!\n", drive->name,
> > +				(ireason & IO) ? "Write" : "Read",
> > +				(ireason & IO) ? "Read" : "Write");
> > +		return ide_do_reset(drive);
> > +	}
> > +	if (!(pc->flags & PC_FLAG_WRITING)) {
> >  		temp = pc->xferred + bcount;
> >  		if (temp > pc->req_xfer) {
> >  			if (temp > pc->buf_size) {
> > @@ -436,7 +444,6 @@ static ide_startstop_t idescsi_pc_intr (
> >  					"- discarding data\n");
> >  				temp = pc->buf_size - pc->xferred;
> >  				if (temp) {
> > -					pc->flags &= ~PC_FLAG_WRITING;
> >  					if (pc->sg)
> >  						idescsi_input_buffers(drive, pc,
> >  									temp);
> > @@ -457,15 +464,11 @@ static ide_startstop_t idescsi_pc_intr (
> >  			printk (KERN_NOTICE "ide-scsi: The scsi wants to send us more data than expected - allowing transfer\n");
> >  #endif /* IDESCSI_DEBUG_LOG */
> >  		}
> > -	}
> > -	if (ireason & IO) {
> > -		pc->flags &= ~PC_FLAG_WRITING;
> >  		if (pc->sg)
> >  			idescsi_input_buffers(drive, pc, bcount);
> >  		else
> >  			hwif->input_data(drive, NULL, pc->cur_pos, bcount);
> >  	} else {
> > -		pc->flags |= PC_FLAG_WRITING;
> 
> Yeah, what was the driver doing turning on/off that flag in the irq handler? Are
> those pc's getting reused in ide-scsi... I guess this is one of the bugs you
> were talking about :).

I guess that it was a left-over from the old days...

[ nowadays idescsi_queue() dynamically allocates new pc-s ]

Thanks,
Bart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/