Messages in this thread | | | Date | Thu, 8 Mar 2018 15:18:24 +0100 | From | Petr Mladek <> | Subject | Re: [PATCH] vsprintf: Make "null" pointer dereference more robust |
| |
On Wed 2018-03-07 10:34:17, Linus Torvalds wrote: > On Wed, Mar 7, 2018 at 7:52 AM, Petr Mladek <pmladek@suse.com> wrote: > > If we are changing things, let's do it properly. The range > > (-PAGE_SIZE,+PAGE_SIZE) is just a small subset of invalid pointers. > > Let's try to catch more of them by reading one byte using > > probe_kernel_read(). It would return -FAULT if we are not able > > to read the address but it would not crash. > > > > Then we clearly need a new message when dereferencing invalid > > poitners that are not pure NULL. I propose (efault). > > "probe_kernel_read()" is really complicated. It takes a *fault* for chrissake! > > Guess what happens now to any crash report if it uses %p and there is > anything wrong with the VM?
This patch does _not_ affect plain %p, %px, and %pK!
It affects %s and %p* modifiers that need to read data from the given address.
> Yes, yes, it disables page faults, but that only means that we won't > go all the way into the generic VM. We'll still take the fauly, still > do vmalloc fault filling, still do a *lot* of potentially really > complicated things.
But the faulty way would happen anyway when vsnprintf() tried to access the data at the given address.
> Guys, stop this idiocy. printk() needs to be *simple* and *reliable*, not fancy.
This patch is primary about printk() reliability. We want a message instead of a silent crash.
I am open for better ideas how to get the fault-related messages out when logbug_lock is taken. At the moment we have them in printk_safe per-CPU buffer. The only chance to see them is crashdump or printk_safe_flush_on_panic() called in single CPU mode.
> Plus, you just made %p be an excellent leak of some very sensitive > information, like "where is the kernel mapped" etc.
We might call panic() after lockbuf_lock is released. This would help to see the message and prevent crash-free hunting for kernel location.
Best Regards, Petr
| |