On Fri, Jun 15, 2018 at 04:47:47PM -0400, Douglas Gilbert wrote:

On 2018-06-15 12:40 PM, Al Viro wrote:

quoted

On Fri, Jun 15, 2018 at 05:23:35PM +0200, Jann Horn wrote:

quoted

I've mostly copypasted ib_safe_file_access() over as
scsi_safe_file_access() because I couldn't find a good common header -
please tell me if you know a better way.
The duplicate pr_err_once() calls are so that each of them fires once;
otherwise, this would probably have to be a macro.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: <redacted>
Signed-off-by: Jann Horn <jannh@google.com>
---

WTF do you mean, in ->release()?  That makes no sense whatsoever -
what kind of copy_{to,from}_user() would be possible in there?

The folks responsible are no longer active in kernel development ***
but as far as I know the async write(command), read(response) were
added to bsg over 10 years ago as proof-of-concept and never properly
worked in this async mode. The biggest design problem with it that I'm
aware of is that two tasks which issue write(commands) at about the
same time to the same device can receive one another's read(response).
The tracking of which response belongs to which task is in part the
reason why the sg driver's data structures are more complex than the
bsg driver's are. Hence the code work to fix that problem in the bsg
driver is not trivial and probably a reason why no-one has attempted it.

Once real world users (needing an async SCSI (or general storage)
pass-through) find out about that bsg "feature", they don't use it.
They use the sg driver or something else. So the fact that bsg has
other glaring errors in it in async mode is no surprise to me.

When I took over the maintenance of the sg driver in 1998, it only
had an async (i.e. write(command), read(response)) interface.
The SG_IO ioctl was added at the suggestion of J�rg Schilling (of
cdrecord "fame"). The sg driver implementation was essentially to
put a write(command) and read(response) back-to-back. The bsg driver
came along later and started with the synchronous SG_IO ioctl
interface only. The async write(command)/read(response) functionality
was added later to bsg. Perhaps that part of the bsg driver should be
deprecated/withdrawn if a maintainer/rewriter cannot be found.
[BTW the bsg sync SG_IO ioctl implementation can probably get the
wrong response, it's just that the window is a lot narrower.]

That said, the bsg driver has lots of other users. For example it is
the only generic pass-through in Linux for the SAS Management Protocol
(SMP) used to control SAS based storage enclosures. I have a user space
package based on it (in Linux) called smp_utils which works well IMO.
However disk enclosures won't typically have contention between users
trying to control them and I'm not aware of any disk enclosures that
support Persistent Reservations. So the bsg driver's "async" problems
are not a practical issue in this case. Also I believe some high end
storage hardware uses bsg to communicate with their hardware from their
user space tools.

We definitely use the BSG IOCTL part for FC-Command passthrough (and
other SCSI transports need it for theirs too, SAS, iSCSI, ...). And I am
not aware of any other way to do that right now. So this can not go away
without someone giving us a different way to do that.

That said.. the read()/write() interface is just pointless atm, I don't
see any sane way of using that in userspace.

FWIW, I actually thought about rewriting that part so it becomes
somewhat sane (tracking which thread sends what command and so on), but
haven't had time to really doing it. With the whole discussion now, I am
not sure anyone really needs that anyway.


- Benjamin

--
With Best Regards, Benjamin Block      /      Linux on IBM Z Kernel Development
IBM Systems & Technology Group   /  IBM Deutschland Research & Development GmbH
Vorsitz. AufsR.: Martina Koederitz       /      Gesch�ftsf�hrung: Dirk Wittkopp
Sitz der Gesellschaft: B�blingen / Registergericht: AmtsG Stuttgart, HRB 243294