Re: [PATCH] /dev/mem: Bail out upon SIGKILL when reading memory.

[Ingo Molnar]

* Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx> wrote:

> >  - We should probably separate out a third 'fatal error' variant: for 
> >    example if copying to user-space generates a page fault, then we 
> >    clearly should not pretend that all is fine and return a short read 
> >    even if we made some progress, a -EFAULT is more informative, as we 
> >    might have corrupted (overran) some user buffer on the failed copy as 
> >    well, and ran off the end into the first unmapped user area.
> 
> Is it possible that copy_from_user() corrupts user buffer in a way that userspace
> cannot retry when kernel responded with "there was a short write"? It seems that
> these functions are difficult to return appropriate errors...

In the cleanest implementation I believe should be done is to 
differentiate between 'kernel side errors' and 'user side errors':

 - 'kernel side errors' are conditions that relate to the layout of 
   kernel memory: some areas might not be readable, and there might be 
   cachability restrictions - or the kernel ran out of memory. In this 
   case it's not user-space's "fault" that they ran into an error and 
   returning a partial read might improve the whole read process, as 
   user-space can decide to continue reading at the last offset that was 
   read - and would also be able to extract all information that can be 
   extracted.

 - 'user side errors' on the other hand are conditions that are probably 
   a user-space bug: such as trying to read() too much data into a too 
   small buffer, running off the end of it and potentially generating a 
   -EFAULT. In this case the kernel should not return a short read, but 
   escalate the error immediately - bugs are easier to find the earlier 
   the condition is reported.

So this is why I think it would make sense to have two error labels: 
"failure" and "fatal_failure". The "failure" case would return a partial 
read if possible (and an error if not), the "fatal_failure" would return 
an error immediately.

This is probably a tad over-engineered, but since you asked ... ;-)

Thanks,

	Ingo