Re: [PATCH V11 0/5] hash addresses printed with %p
On (11/30/17 19:26), Sergey Senozhatsky wrote: > On (11/30/17 10:23), David Laight wrote: > [..] > > > Maybe I'm being thick, but... if we're rendering these addresses > > > unusable by hashing them, why not just print something like > > > "" in their place? That loses the uniqueness thing but I > > > wonder how valuable that is in practice? > > > > My worry is that is you get a kernel 'oops' print with actual register > > values you have no easy way of tying an address or address+offset to > > the corresponding hash(address) printed elsewhere. > > print the existing hash:pointer mappings in panic()? [if we can do that] by this I meant "when oops_in_progress == 1 then print hash:pointer for %p, not just hash". -ss
Re: [PATCH V11 0/5] hash addresses printed with %p
On (11/30/17 10:23), David Laight wrote: [..] > > Maybe I'm being thick, but... if we're rendering these addresses > > unusable by hashing them, why not just print something like > > "" in their place? That loses the uniqueness thing but I > > wonder how valuable that is in practice? > > My worry is that is you get a kernel 'oops' print with actual register > values you have no easy way of tying an address or address+offset to > the corresponding hash(address) printed elsewhere. print the existing hash:pointer mappings in panic()? [if we can do that] -ss
RE: [PATCH V11 0/5] hash addresses printed with %p
From: Andrew Morton > Sent: 29 November 2017 23:21 > > > > The added advantage of hashing %p is that security is now opt-out, if > > you _really_ want the address you have to work a little harder and use > > %px. You need a system-wide opt-out that prints the actual values. Otherwise developers will use something else to print addresses and the code will remain in the released drivers. > > The idea for creating the printk specifier %px to print the actual > > address was suggested by Kees Cook (see below for email threads by > > subject). > > Maybe I'm being thick, but... if we're rendering these addresses > unusable by hashing them, why not just print something like > "" in their place? That loses the uniqueness thing but I > wonder how valuable that is in practice? My worry is that is you get a kernel 'oops' print with actual register values you have no easy way of tying an address or address+offset to the corresponding hash(address) printed elsewhere. David
Re: [PATCH V11 0/5] hash addresses printed with %p
On Wed, Nov 29, 2017 at 03:20:40PM -0800, Andrew Morton wrote: > On Wed, 29 Nov 2017 13:05:00 +1100 "Tobin C. Harding" wrote: > > > Currently there exist approximately 14 000 places in the Kernel where > > addresses are being printed using an unadorned %p. This potentially > > leaks sensitive information regarding the Kernel layout in memory. Many > > of these calls are stale, instead of fixing every call lets hash the > > address by default before printing. This will of course break some > > users, forcing code printing needed addresses to be updated. We can add > > a printk specifier for this purpose (%px) to give developers a clear > > upgrade path for breakages caused by applying this patch set. > > > > The added advantage of hashing %p is that security is now opt-out, if > > you _really_ want the address you have to work a little harder and use > > %px. > > > > The idea for creating the printk specifier %px to print the actual > > address was suggested by Kees Cook (see below for email threads by > > subject). > > Maybe I'm being thick, but... if we're rendering these addresses > unusable by hashing them, why not just print something like > "" in their place? That loses the uniqueness thing but I > wonder how valuable that is in practice? The discussion on this has been fragmented over _at least_ 5 patch sets with totally different subjects. And I only just added you to the CC list, my apologies if this is a bit confusing. Consensus was that if we provide a unique identifier (the hashed address) then this is useful for debugging (i.e differentiating between structs when you have a list of them). The first 32 bits (on 64 bit machines) were zeroed out because 1. they are unnecessary in achieving the aim. 2. it reduces noise. 3. makes explicit some funny business was going on. And bonus points, hopefully we don't break userland tools that parse addresses if the format is still the same. thanks, Tobin.
Re: [PATCH V11 0/5] hash addresses printed with %p
On Wed, 29 Nov 2017 13:05:00 +1100 "Tobin C. Harding" wrote: > Currently there exist approximately 14 000 places in the Kernel where > addresses are being printed using an unadorned %p. This potentially > leaks sensitive information regarding the Kernel layout in memory. Many > of these calls are stale, instead of fixing every call lets hash the > address by default before printing. This will of course break some > users, forcing code printing needed addresses to be updated. We can add > a printk specifier for this purpose (%px) to give developers a clear > upgrade path for breakages caused by applying this patch set. > > The added advantage of hashing %p is that security is now opt-out, if > you _really_ want the address you have to work a little harder and use > %px. > > The idea for creating the printk specifier %px to print the actual > address was suggested by Kees Cook (see below for email threads by > subject). Maybe I'm being thick, but... if we're rendering these addresses unusable by hashing them, why not just print something like "" in their place? That loses the uniqueness thing but I wonder how valuable that is in practice?
