linux-kernel - Re: [ide] b7fb14d3ac: EIP:ioread32

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]

Message-ID: <20210706143647.GA28289@lst.de>
Date:   Tue, 6 Jul 2021 16:36:47 +0200
From:   Christoph Hellwig <hch@....de>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Christoph Hellwig <hch@....de>,
        kernel test robot <oliver.sang@...el.com>,
        Jens Axboe <axboe@...nel.dk>,
        LKML <linux-kernel@...r.kernel.org>, lkp@...ts.01.org,
        kernel test robot <lkp@...el.com>,
        "H. Peter Anvin" <hpa@...or.com>, Borislav Petkov <bp@...en8.de>
Subject: Re: [ide] b7fb14d3ac: EIP:ioread32_rep

On Mon, Jul 05, 2021 at 01:00:09PM -0700, Linus Torvalds wrote:
> It might perhaps be worth adding some kind of
> 
>      WARN_ON_ONCE(offset & 511);
> 
> in the callchain somewhere for that ata_queued_cmd(). But at that
> point I no longer know the code.

Yeah, there's usually a huge offset into the page.  The otherwise
similar ATAPI code actually has checks to chunk it up and not cross
page boundaries, and copying that over fixes the problem.  The testbot
config then crashes in an infiband driver, but disabling that driver
entirely makes it boot fine:

diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index ae7189d1a568..018f021c45a3 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -651,7 +651,7 @@ static void ata_pio_sector(struct ata_queued_cmd *qc)
 	int do_write = (qc->tf.flags & ATA_TFLAG_WRITE);
 	struct ata_port *ap = qc->ap;
 	struct page *page;
-	unsigned int offset;
+	unsigned int offset, count;
 	unsigned char *buf;
 
 	if (!qc->cursg) {
@@ -670,16 +670,22 @@ static void ata_pio_sector(struct ata_queued_cmd *qc)
 
 	DPRINTK("data %s\n", qc->tf.flags & ATA_TFLAG_WRITE ? "write" : "read");
 
+	/* don't overrun current sg */
+	count = min(qc->cursg->length - qc->cursg_ofs, qc->sect_size);
+
+	/* don't cross page boundaries */
+	count = min(count, (unsigned int)PAGE_SIZE - offset);
+
 	/* do the actual data transfer */
 	buf = kmap_atomic(page);
-	ap->ops->sff_data_xfer(qc, buf + offset, qc->sect_size, do_write);
+	ap->ops->sff_data_xfer(qc, buf + offset, count, do_write);
 	kunmap_atomic(buf);
 
 	if (!do_write && !PageSlab(page))
 		flush_dcache_page(page);
 
-	qc->curbytes += qc->sect_size;
-	qc->cursg_ofs += qc->sect_size;
+	qc->curbytes += count;
+	qc->cursg_ofs += count;
 
 	if (qc->cursg_ofs == qc->cursg->length) {
 		qc->cursg = sg_next(qc->cursg);