linux-kernel - Re: [PATCHSET RFC 0/437] Kill off old fops ->read() and ->write()

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]

Date: Sat, 13 Apr 2024 05:15:34 +0100
From: Al Viro <viro@...iv.linux.org.uk>
To: Jens Axboe <axboe@...nel.dk>
Cc: linux-kernel@...r.kernel.org
Subject: Re: [PATCHSET RFC 0/437] Kill off old fops ->read() and ->write()

On Fri, Apr 12, 2024 at 07:58:13AM -0600, Jens Axboe wrote:

> I'm aware of some drivers that do different things from write vs writev,
> or read vs readv for instance. But those I did cater to, by having a
> flag they can now check.
> 
> Can you be a bit more specific on an example of a driver that does the
> above?

Consider e.g. your #39.  Current mainline: 1 call of ->set() for each
segment passed to writev() on any of those.  With your patch: call
segments concatenated and if the concatenation looks like a number,
a single call of ->set().

If nothing else, it's a user-visible ABI change.  And in cases when ->set()
has non-trivial side effects, it just might break a real-world code that
is currently correct.

I picked that one because I didn't want to dig through the drivers -
I'm pretty sure that there's more to be found there.

It's not just "write() and writev() parse the data in different way" - we do
have a couple of those, but that's a minor problem.

"write(fd, buf, len1); write(fd, buf + len1, len1 + len2); is not the same
thing as write(fd, buf, len1 + len2)" is not rare for character devices and
for regular files on procfs/debugfs/etc.

For any of those you need to use you vfs_write_iter() helper or you'll
be breaking userland ABI.  The cost of audit of several thousands of ->write()
(and ->read() - similar problem applies there) instances, checking that property
is the main reason this conversion hadn't been already done.