Re: [PATCH v3 17/20] sg: add sg_iosubmit_v3 and sg_ioreceive_v3 ioctls

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 8/12/19 11:37 AM, Douglas Gilbert wrote:
> On 2019-08-09 7:15 p.m., James Bottomley wrote:
>> On Wed, 2019-08-07 at 13:42 +0200, Douglas Gilbert wrote:
>>> Add ioctl(SG_IOSUBMIT_V3) and ioctl(SG_IORECEIVE_V3). These ioctls
>>> are meant to be (almost) drop-in replacements for the write()/read()
>>> async version 3 interface. They only accept the version 3 interface.
>> I don't think we should do this at all.  Anyone who wants to use the
>> new async interfaces should use the v4 headers.  As Tony Battersby
>> already said, the legacy v3 users aren't going to update, so there's no
>> point at all introducing new interfaces for v3.  We simply keep the v3
>> only read/write interface until there are no users left and it can be
>> eliminated.
> Tony Battersby wrote [20190809]:
>    "Actually I used the asynchronous write()/read()/poll() sg interface to
>    implement RAID-like functionality for tape drives and medium changers,
>    in a commercial product that has been around since 2002.  These days our
>    products use a lot more disk I/O than tape I/O, so I don't write much
>    new code using the sg interface anymore, although that code is still
>    there and has to be maintained as needed.  So I don't have any immediate
>    plans to use any of the new sgv4 features being introduced, and
>    unfortunately I am way too busy to even give it a good review..."
>
> That is quoted in full his post. And here is the only other post from
> Tony I can find on this subject, again quoted in full [20190808]:
>
>    "One of the reasons ioctls have a bad reputation is because they can be
>    used to implement poorly-thought-out interfaces.  So kernel maintainers
>    push back on adding new ioctls.  But the push back isn't about the
>    number of ioctls, it is about the poor interfaces.  My advice was that
>    in general, to implement a given API, it would be better to add more
>    ioctls with a simple interface for each one rather than to add fewer
>    extremely complex multiplexing ioctls."
>
> Call me biased but I believe that taken together those posts support
> what I am proposing. And I can _not_ see how you deduce: "so there's
> no point at all introducing new interfaces for v3" in reference to
> Tony's posts.
>
>
> As I stated in a previous post, I do not consider the sg v3 interface
> as legacy. Where simply implemented, I am prepared to implement new
> features on both the sg v3 and v4 interfaces. One example of this is
> doing command timing in nanoseconds rather than the current default,
> which is timing in milliseconds. There is also the new option of not
> doing any command timing at all. In my current implementation it would
> actually be more code to implement that for the v4 interface but not
> for the v3 interface.
>
> Replicating my argument from a previous post:
> If the kernel had an API mapping layer that was sensitive to file
> descriptors of a "special file" (e.g. "/dev/sg3") then it could map:
>      write(sg_fd, &sg_io_v3_obj, sizeof(sg_io_v3_obj))
> to
>      ioctl(sg_fd, SG_IOSUBMIT_V3, &sg_io_v3_obj)
>
> Plus a similar mapping for read() to ioctl(SG_IORECEIVE_V3). If such
> a mapping did exist and was transparent to the user then write()
> and read() could be retired from the sg driver.  And I assume that
> would get a thumbs up from the kernel security folk.
>
FWIW, my employer will probably continue to use the async sg v3
interface for a long time.  If the read/write syscalls are a security
problem, and if we had ioctl()s that are mostly a drop-in replacement
for them, then we could convert our products over to the new ioctl()s on
our next kernel upgrade without too much work (our products are embedded
devices, so we control the whole software stack).  So if you plan to
deprecate the read/write syscall interface anytime soon, then having
drop-in replacement ioctl()s would be beneficial, even if it can't be
done transparently as Doug suggests.

Tony Battersby
Cybernetics




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux