* set_fs(KERNEL_DS) vs iovec
@ 2018-05-22 5:08 Benjamin Herrenschmidt
2018-05-22 10:20 ` Christoph Hellwig
0 siblings, 1 reply; 3+ messages in thread
From: Benjamin Herrenschmidt @ 2018-05-22 5:08 UTC (permalink / raw)
To: Al Viro, Greg Kroah-Hartman, Greg Kroah-Hartman; +Cc: linux-kernel
Hi guys !
I was helping with a small driver when I stumbled upon a comment from a
reviwer pointing to an old lwn article talking about deprecating set_fs
due to security concerns:
https://lwn.net/Articles/722267/
Now, this is a very simple driver running on a small/slow ARM SoC,
which reads from a FIFO and writes into a destination buffer.
It provides 2 interfaces, a userspace one (read syscall) and an in-
kernel one since some other kernel drivers use it as a transport.
My existing implementation uses the good old construct of doing
put_user() in the low level FIFO pumping code, and have the in-kernel
API do:
old_fs = get_fs();
set_fs(KERNEL_DS);
rc = __sbefifo_submit(sbefifo, command, cmd_len,
response, resp_len);
set_fs(old_fs);
Which is simple, turns into fairly efficient code on that simple
device, and doesn't seem to have security issues...
That said, following the advice in that article, I tried to look at the
iovec stuff and noticed:
- The APIs are almost entirely undocumented (or did I look in the
wrong place ?)
- The code in lib/iov_iter.c is rather ... unreadable
- It's also significantly more complex, thus would probably result in
a slower driver (remember: small SoC). It's quite overkill for my
simple use case.
- There are very few users, set_fs(KERNEL_DS) is still the most common
method used by drivers.
Hence my question: Is is still acceptable these days to use
set_fs(KERNEL_DS) for simple cases like this ? Or is it really
deprecated and all new users should use the iovec's ?
Cheers,
Ben.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: set_fs(KERNEL_DS) vs iovec
2018-05-22 5:08 set_fs(KERNEL_DS) vs iovec Benjamin Herrenschmidt
@ 2018-05-22 10:20 ` Christoph Hellwig
2018-05-22 12:44 ` Benjamin Herrenschmidt
0 siblings, 1 reply; 3+ messages in thread
From: Christoph Hellwig @ 2018-05-22 10:20 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Al Viro, Greg Kroah-Hartman, linux-kernel
On Tue, May 22, 2018 at 03:08:35PM +1000, Benjamin Herrenschmidt wrote:
> Hence my question: Is is still acceptable these days to use
> set_fs(KERNEL_DS) for simple cases like this ?
Not at all.
> Or is it really
> deprecated and all new users should use the iovec's ?
Yes, please use iov_iter. It should actually make your code simpler,
too.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: set_fs(KERNEL_DS) vs iovec
2018-05-22 10:20 ` Christoph Hellwig
@ 2018-05-22 12:44 ` Benjamin Herrenschmidt
0 siblings, 0 replies; 3+ messages in thread
From: Benjamin Herrenschmidt @ 2018-05-22 12:44 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Al Viro, Greg Kroah-Hartman, linux-kernel
On Tue, 2018-05-22 at 03:20 -0700, Christoph Hellwig wrote:
> On Tue, May 22, 2018 at 03:08:35PM +1000, Benjamin Herrenschmidt wrote:
> > Hence my question: Is is still acceptable these days to use
> > set_fs(KERNEL_DS) for simple cases like this ?
>
> Not at all.
>
> > Or is it really
> > deprecated and all new users should use the iovec's ?
>
> Yes, please use iov_iter. It should actually make your code simpler,
> too.
In this case it really doesn't :-) But that doesn't matter, I get the
message, I'll fix the driver accordingly.
Cheers,
Ben.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-05-22 12:44 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-22 5:08 set_fs(KERNEL_DS) vs iovec Benjamin Herrenschmidt
2018-05-22 10:20 ` Christoph Hellwig
2018-05-22 12:44 ` Benjamin Herrenschmidt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).