linux-kernel - Re: [PATCH] BUG: nr_phys_segments cannot be less than nr_hw

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, 7 Oct 2008 02:24:29 +0900
From:	FUJITA Tomonori <fujita.tomonori@....ntt.co.jp>
To:	jens.axboe@...cle.com
Cc:	James.Bottomley@...senPartnership.com, knikanth@...e.de,
	linux-kernel@...r.kernel.org, linux-scsi@...r.kernel.org,
	fujita.tomonori@....ntt.co.jp
Subject: Re: [PATCH] BUG: nr_phys_segments cannot be less than
 nr_hw_segments

On Thu, 2 Oct 2008 19:13:57 +0200
Jens Axboe <jens.axboe@...cle.com> wrote:

> On Thu, Oct 02 2008, James Bottomley wrote:
> > On Thu, 2008-10-02 at 18:58 +0200, Jens Axboe wrote:
> > > On Thu, Oct 02 2008, James Bottomley wrote:
> > > > The bug would appear to be that we sometimes only look at q->max_sectors
> > > > when deciding on mergability.  Either we have to insist on max_sectors
> > > > <= hw_max_sectors, or we have to start using min(q->max_sectors,
> > > > q->max_hw_sectors) for this.
> > > 
> > > q->max_sectors MUST always be <= q->max_hw_sectors, otherwise we could
> > > be sending down requests that are too large for the device to handle. So
> > > that condition would be a big bug. The sysfs interface checks for this,
> > > and blk_queue_max_sectors() makes sure that is true as well.
> > 
> > Yes, that seems always to be enforced.  Perhaps there are other ways of
> > tripping this problem ... I'm still sure if it occurs it's because we do
> > a physical merge where a virtual merge is forbidden.
> > 
> > > The fixes proposed still look weird. There is no phys vs hw segment
> > > constraints, the request must adhere to the limits set by both. It's
> > > mostly a moot point anyway, as 2.6.28 will get rid of the hw accounting
> > > anyway.
> > 
> > Agree all three proposed fixes look wrong.  However, if it's what I
> > think, just getting rid of hw accounting won't fix the problem because
> > we'll still have the case where a physical merge is forbidden by iommu
> > constraints ... this still needs to be accounted for.
> > 
> > What we really need is a demonstration of what actually is going
> > wrong ...
> 
> Yep, IIRC we both asked for that the last time as well... Nikanth?

Possibly, blk_phys_contig_segment might miscalculate
q->max_segment_size?

blk_phys_contig_segment does:

req->biotail->bi_size + next_req->bio->bi_size > q->max_segment_size;

But it's possible that req->biotail and the previous bio are supposed
be merged into one segment? Then we could create too large segment
here.
--
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/