Messages in this thread | | | Date | Mon, 26 Aug 2019 13:19:44 +0200 | From | Ingo Molnar <> | Subject | Re: [PATCH] /dev/mem: Bail out upon SIGKILL when reading memory. |
| |
* Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote:
> On 2019/08/26 1:54, Linus Torvalds wrote: > > On Sat, Aug 24, 2019 at 10:50 PM Tetsuo Handa > > <penguin-kernel@i-love.sakura.ne.jp> wrote: > >> > >> @@ -142,7 +144,7 @@ static ssize_t read_mem(struct file *file, char __user *buf, > >> sz = size_inside_page(p, count); > >> cond_resched(); > >> err = -EINTR; > >> - if (fatal_signal_pending(current)) > >> + if (signal_pending(current)) > >> goto failed; > >> > >> err = -EPERM; > > > > So from a "likelihood of breaking" standpoint, I'd really like to make > > sure that the "signal_pending()" checks come at the *end* of the loop. > > > > That way, if somebody is doing a 4-byte read from MMIO, he'll never see -EINTR. > > > > I'm specifically thinking of tools like user-space 'lspci' etc, which > > I wouldn't be surprised could happen. > > > > Also, just in case things break, I do agree with Ingo that this should > > be split up into several patches. > > Thinking from how read_mem() returns error code instead of returning bytes > already processed, any sane users will not try to read so much memory (like 2GB). > If userspace programs want to read so much memory, there must have been attempts > to improve performance. I guess that userspace program somehow knows which region > to read and tries to read only meaningful pages (which would not become hundreds MB). > Thus, I don't think we want to make /dev/{mem,kmem} intrruptible. Just making killable > in case insane userspace program (like fuzzer) tried to read/write so much memory > will be sufficient...
Basically making IO primitives interruptible is the norm and it's a quality of implementation issue: it's only a historic accident that /dev/mem read()s aren't.
So let's try and make it interruptible as the #3 patch I sent did - of course if anything breaks we'll have to undo it. But if we can get away with then by all means let's do so - even shorter reads can generate nasty long processing latencies.
Ok?
Thanks,
Ingo
| |