linux-kernel - Re: [PATCHv3] block: Add support for Sony SxS cards

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>] [day] [month] [year] [list]

Date:	Sun, 28 Sep 2014 10:35:44 -0600
From:	Jens Axboe <axboe@...nel.dk>
To:	Kieran Kunhya <kieran@...hya.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"David S. Miller" <davem@...emloft.net>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Joe Perches <joe@...ches.com>,
	Mauro Carvalho Chehab <m.chehab@...sung.com>,
	Antti Palosaari <crope@....fi>,
	Jingoo Han <jg1.han@...sung.com>,
	Ramprasad Chinthekindi <rchinthekindi@...c-inc.com>,
	Pekka Enberg <penberg@...nel.org>,
	Minchan Kim <minchan@...nel.org>,
	Fabian Frederick <fabf@...net.be>,
	Akhil Bhansali <abhansali@...c-inc.com>,
	Jiri Kosina <jkosina@...e.cz>,
	Alan <gnomes@...rguk.ukuu.org.uk>
CC:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCHv3] block: Add support for Sony SxS cards

On 09/28/2014 08:39 AM, Kieran Kunhya wrote:
>> So a few questions here... This device only does reads? And it seems to 
>> be assuming that only reads end up in the request handler? How so?
> 
> I have only reverse engineered the read-part of the device. Yes, the 
> driver should check whether the request is a read. 

And you should also deny any writeable opens, in that case. Add an open
method and check for FMODE_WRITE. Then add a check ala:

	if (bio_data_dir(bio)) {
		bio_endio(bio, -EROFS);
		return;
	}

and the top of your sxs_request().

>> Second question. IO never fails? There's no status checking and IO is 
>> always ended successfully. This too seems odd.
> 
> I will fix this.
> 
>> Third, this wong work as-is, at least not of HIGHMEM is used. After the 
>> bvec_kmap_irq(), irqs will be disabled. But your sxs_memcpy_read() 
>> invokes schedule through the completion waits, that will instantly go bad.
> 
> Is there a better way of doing this? I spent a long time trying to get DMA 
> to work but couldn't so had to resort to this ugly hack. I assume the memcpy 
> also needs to have some locking somewhere?

It really is pretty horrible and slow, but if you can't get DMA working,
then so be it. And yes, you should add a mutex that is held for the
duration of the memcpy_read(), otherwise things will go bad very fast if
two or more processes attempt to read from it at the same time. A quick
work-around for the irq issue would be to have the block layer bounce
the highmem pages for you. That's actually the default behavior, but I
would make it explicit with a call to:

	blk_queue_bounce_limit(q, BLK_BOUNCE_HIGH);

after the blk_queue_make_request() call. If you do that, then get rid of
the bvec_kmap_irq(), and just do:

	buffer = page_address(bvec.bv_page) + bvec.bv_offset;

to get the destination for your read.

-- 
Jens Axboe

--
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/