Messages in this thread | | | From | Kees Cook <> | Date | Wed, 29 Nov 2017 13:31:07 -0800 | Subject | Re: [GIT PULL] hash addresses printed with %p |
| |
On Wed, Nov 29, 2017 at 11:39 AM, Linus Torvalds <torvalds@linux-foundation.org> wrote: > On Wed, Nov 29, 2017 at 11:22 AM, Linus Torvalds > <torvalds@linux-foundation.org> wrote: >> >> What I didn't realize until after pulling this and testing, is that it >> completely breaks '%pK'. >> >> We've marked various sensitive pointers with %pK, but that is now >> _less_ secure than %p is, since it doesn't do the hashing because of >> how you refactored the %pK code out of 'pointer()' into its own >> function. >> >> So now %pK ends up using the plain "number()" function. Reading >> through the series I hadn't noticed that the refactoring ended up >> messing with that. >> >> I'll fix it up somehow. > > I ended up just doing this: > > case 'K': > + if (!kptr_restrict) > + break; > return restricted_pointer(buf, end, ptr, spec); > > which basically says that "if kptr_restrict isn't set, %pK is the same as %p". > > Now, I feel that we should probably get rid of 'restricted_pointer()' > entirely, since now the regular '%p' is arguably safer than '%pK' is, > but I also didn't want to mess with the case that I have never used > and that most distros don't seem to set.
kptr_restrict=0 is now much safer, yes (i.e. %pK becomes hashed %p). Strictly speaking, kptr_restrict > 0 is "better" than hashed %p, in that it only says "0".
> Alternatively, we might make the 'K' behavior of clearing the pointer > be in addition to the other flags, so that you could do '%pxK' and get > the old %pK behavior. But since I am not a huge fan of %pK to begin > with, I can't find it in myself to care too much. > > So I'll leave that for Kees & co to decide on. Comments?
I'm not hugely attached to %pK, but retaining its ability to zero out the result would be nice.
(If we in the future we have a toggle for %p that switches it from hashing to zeroing, then we could entirely drop %pK.)
-Kees
-- Kees Cook Pixel Security
| |