linux-kernel - [RFC] weird semantics of SG_DXFER_TO_FROM_DEV in BLK_DEV

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

Message-ID: <20160404033845.GE17997@ZenIV.linux.org.uk>
Date:	Mon, 4 Apr 2016 04:38:45 +0100
From:	Al Viro <viro@...IV.linux.org.uk>
To:	Jens Axboe <axboe@...com>
Cc:	linux-kernel@...r.kernel.org, linux-block@...r.kernel.org
Subject: [RFC] weird semantics of SG_DXFER_TO_FROM_DEV in BLK_DEV_SKD
 (drivers/block/skd*)

	SG_DXFER_TO_FROM_DEV is weird everywhere, but skd manages to get it
even stranger than usual: copying to/from userland happens in
skd_sg_io_copy_buffer(), which is called twice - once before the actual
talking to device and once after.  The first call either does nothing or
copies from userland, the second one either does nothing or copies to
userland.  So far, so good, but the logics in that sucker deciding whether
to bugger off or not is wrong:
        if (dxfer_dir != sksgio->sg.dxfer_direction) {
                if (dxfer_dir != SG_DXFER_TO_DEV ||
                    sksgio->sg.dxfer_direction != SG_DXFER_TO_FROM_DEV)
                        return 0;
        }

The first call is with dxfer_dir equal to SG_DXFER_TO_DEV, the second -
SG_DXFER_FROM_DEV.  So we get

SG_DXFER_NONE:		nothing		nothing
SG_DXFER_FROM_DEV:	nothing		out
SG_DXFER_TO_DEV:	in		nothing
SG_DXFER_TO_FROM_DEV:	in		nothing

I've no idea if anything is still using SG_DXFER_TO_FROM_DEV, but this
behaviour AFAICS doesn't match that of write() on /dev/sg* (both
in and out are done) or normal SG_IO (either both in and out, in case if it
hits bio_map_user_iov(), or only out if it hits bio_copy_user_iov()).  In
all cases the out part is done.  Here it is skipped.

Not sure who (if anybody) maintains it these days, but that behaviour looks
wrong...