Messages in this thread | | | Subject | Re: [PATCH] /dev/mem: Bail out upon SIGKILL when reading memory. | From | Tetsuo Handa <> | Date | Thu, 22 Aug 2019 18:59:25 +0900 |
| |
Tetsuo Handa wrote: > Greg Kroah-Hartman wrote: > > Oh, nice! This shouldn't break anything that is assuming that the read > > will complete before a signal is delivered, right? > > > > I know userspace handling of "short" reads is almost always not there... > > Since this check will give up upon SIGKILL, userspace won't be able to see > the return value from read(). Thus, returning 0 upon SIGKILL will be safe. ;-) > Maybe we also want to add cond_resched()... > > By the way, do we want similar check on write_mem() side? > If aborting "write to /dev/mem" upon SIGKILL (results in partial write) is > unexpected, we might want to ignore SIGKILL for write_mem() case. > But copying data from killed threads (especially when killed by OOM killer > and userspace memory is reclaimed by OOM reaper before write_mem() returns) > would be after all unexpected. Then, it might be preferable to check SIGKILL > on write_mem() side... >
Ha, ha. syzbot reported the same problem using write_mem(). https://syzkaller.appspot.com/text?tag=CrashLog&x=1018055a600000 We want fatal_signal_pending() check on both sides.
By the way, write_mem() worries me whether there is possibility of replacing kernel code/data with user-defined memory data supplied from userspace. If write_mem() were by chance replaced with code that does
while (1);
we won't be able to return from write_mem() even if we added fatal_signal_pending() check. Ditto for replacing local variables with unexpected values...
| |