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

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]

Message-ID: <20190825104858.GA119494@gmail.com>
Date:   Sun, 25 Aug 2019 12:48:58 +0200
From:   Ingo Molnar <mingo@...nel.org>
To:     Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Arnd Bergmann <arnd@...db.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Linux List Kernel Mailing <linux-kernel@...r.kernel.org>,
        syzbot <syzbot+8ab2d0f39fb79fe6ca40@...kaller.appspotmail.com>
Subject: Re: [PATCH] /dev/mem: Bail out upon SIGKILL when reading memory.


* Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp> 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