printk: add more new kernel pointer filter options.
Subsystems (7)
Greg KH
maintainercover printk: add more new kernel pointer filter options.
Here's a short patch series from Chris Fries and Dave Weinstein that implement some new restrictions when printing out kernel pointers, as well as the ability to whitelist kernel pointers where needed.
These patches are based on work from William Roberts, and also is inspired by grsecurity's %pP to specifically whitelist a kernel pointer, where it is always needed, like the last patch in the series shows, in the UIO drivers (UIO requires that you know the address, it's a hardware address, nothing wrong with seeing that...)
I haven't done much to this patch series, only forward porting it from an older kernel release (4.4) and a few minor tweaks. It applies cleanly on top of 4.11 as well as Linus's current development tree (10502 patches into the 4.12-rc1 merge window). I'm posting it now for comments if anyone sees anything wrong with this approach, or thinks the things that are being whitelisted should not be?
thanks,
greg k-h
Documentation/printk-formats.txt | 15 +++- Documentation/sysctl/kernel.txt | 11 +++ arch/arm64/kernel/traps.c | 4 - drivers/uio/uio.c | 4 - include/linux/kallsyms.h | 2 kernel/printk/printk.c | 2 kernel/sysctl.c | 6 - lib/vsprintf.c | 128 ++++++++++++++++++++++++++++----------- 8 files changed, 123 insertions(+), 49 deletions(-)
Greg KH
maintainerRFC 1/6 lib: vsprintf: additional kernel pointer filtering options
From: Dave Weinstein <[email protected]>
Add the kptr_restrict setting of 3 which results in both %p and %pK values being replaced by zeros.
Add an additional %pP value inspired by the Grsecurity option which explicitly whitelists pointers for output.
This patch is based on work by William Roberts <[email protected]>
--- Documentation/printk-formats.txt | 5 +++ Documentation/sysctl/kernel.txt | 3 ++ kernel/sysctl.c | 3 +- lib/vsprintf.c | 81 ++++++++++++++++++++++++++-------------- 4 files changed, 63 insertions(+), 29 deletions(-)
Documentation/printk-formats.txt +5 -0
@@ -64,6 +64,11 @@ Kernel Pointers: users. The behaviour of %pK depends on the kptr_restrict sysctl - see Documentation/sysctl/kernel.txt for more details. + %pP 0x01234567 or 0x0123456789abcdef + + For printing kernel pointers which should always be shown, even to + unprivileged users. + Struct Resources: %pr [mem 0x60000000-0x6fffffff flags 0x2200] or
Documentation/sysctl/kernel.txt +3 -0
@@ -392,6 +392,9 @@ values to unprivileged users is a concern. When kptr_restrict is set to (2), kernel pointers printed using %pK will be replaced with 0's regardless of privileges. +When kptr_restrict is set to (3), kernel pointers printed using +%p and %pK will be replaced with 0's regardless of privileges. + ============================================================== l2cr: (PPC only)
kernel/sysctl.c +2 -1
@@ -129,6 +129,7 @@ static unsigned long one_ul = 1;static int one_hundred =100 ;static int one_thousand =1000 ;#ifdef CONFIG_PRINTK +static int three =3 ;static int ten_thousand =10000 ;#endif #ifdef CONFIG_PERF_EVENTS
@@ -830,7 +831,7 @@ static struct ctl_table kern_table[] = { .mode = 0644, .proc_handler = proc_dointvec_minmax_sysadmin, .extra1 = &zero, - .extra2 = &two, + .extra2 = &three, },#endif {
lib/vsprintf.c +53 -28
143 lines not on this page · raw message
Signature · 1 line
2.12.2
Petr Mladek
maintainerRe: [RFC 1/6] lib: vsprintf: additional kernel pointer filtering options
Quoted 16 lines · Greg KH
From: Dave Weinstein <[email protected]>
Add the
kptr_restrictsetting of 3 which results in both %p and %pK values being replaced by zeros.Add an additional %pP value inspired by the Grsecurity option which explicitly whitelists pointers for output.
This patch is based on work by William Roberts
<[email protected]>lib/vsprintf.c +0
-0
lib/vsprintf.c struct printf_spec { · line 403
Quoted 9 lines
lib/vsprintf.c +6
-0@@ -395,6 +395,16 @@ struct printf_spec {#define FIELD_WIDTH_MAX ((1 << 23) - 1)#define PRECISION_MAX ((1 << 15) - 1) +int kptr_restrict__read_mostly ; + +/* +* Always cleanse %p and %pK specifiers +*/ +static inline int kptr_restrict_always_cleanse_pointers (void )
The name of the function is very long and still confusing. It uses the word "always" but there are many types of pointers that are not cleared with this condition, for example %pP, %pa.
Do we need this helper function at all? It is used a weird way, see below.
Quoted 43 lines
lib/vsprintf.c +28
-1+{ +return kptr_restrict >=3 ; +} +static noinline_for_stack char *number(char *buf, char *end, unsigned long long num,struct printf_spec spec)@@ -1576,7 +1588,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr, {const int default_width =2 *sizeof (void *); -if (!ptr && *fmt !='K' ) { +if (!ptr && *fmt !='K' && !kptr_restrict_always_cleanse_pointers ()) {/* * Print (null) with the same width as a pointer so it makes * tabular output look nice. @@ -1657,10 +1669,43 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,va_end (va);return buf; } +case 'N' : +return netdev_bits (buf, end, ptr, fmt); +case 'a' : +return address_val (buf, end, ptr, fmt); +case 'd' : +return dentry_name (buf, end, ptr, spec, fmt); +case 'C' : +return clock (buf, end, ptr, spec, fmt); +case 'D' : +return dentry_name (buf, end, + ((const struct file *)ptr)->f_path.dentry, + spec, fmt); +#ifdef CONFIG_BLOCK +case 'g' : +return bdev_name (buf, end, ptr, spec, fmt); +#endif + +case 'G' : +return flags_string (buf, end, ptr, fmt); +case 'P' : +/* +* an explicitly whitelisted kernel pointer should never be +* cleansed
lib/vsprintf.c char *pointer(const char *fmt, char *buf, char *end, void *ptr, · line 1704
Quoted 10 lines
lib/vsprintf.c +10
-0+*/ +break ; +default : +/* +* plain %p, no extension, check if we should always cleanse and +* treat like %pK. +*/ +if (!kptr_restrict_always_cleanse_pointers ()) +break ; +/* fallthrough */
Using default: in the middle of other cases is pretty confusing and a call for troubles.
lib/vsprintf.c char *pointer(const char *fmt, char *buf, char *end, void *ptr, · line 1708
Quoted 5 lines
lib/vsprintf.c +1
-1case 'K' :switch (kptr_restrict) {case 0 : -/* Always print %pK values */ +/* Always print %p values */
And this is one example. We are never here for %p because we called break in the "default:" case above for kptr_restrict == 0.
lib/vsprintf.c char *pointer(const char *fmt, char *buf, char *end, void *ptr, · line 1728
Quoted 10 lines
lib/vsprintf.c +1
-1break ;case 1 : {const struct cred *cred;@@ -1679,7 +1724,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,* Only print the real pointer value if the current * process has CAP_SYSLOG and is running with the * same credentials it started with. This is because -* access to files is checked at open() time, but %pK +* access to files is checked at open() time, but %p
Same here. This change makes the feeling that we check CAP_SYSLOG even for plain %p but we actually do not do it.
Quoted 6 lines
lib/vsprintf.c +0
-0* checks permission at read() time. We don't want to * leak pointer values if a binary opens a file using lib/vsprintf.c +0
-0* %pK and then elevates privileges before reading it. @@ -1691,33 +1736,13 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr, ptr =NULL ;break ;
lib/vsprintf.c char *pointer(const char *fmt, char *buf, char *end, void *ptr, · line 1745
Quoted 10 lines
lib/vsprintf.c +2
-2} -case 2 : +case 2 :/* restrict only %pK */ +case 3 :/* restrict all non-extensioned %p and %pK */ default : -/* Always print 0's for %pK */ ptr =NULL ;break ; }break ;
I would personally write this piece of code a more straightforward way, for example:
switch (*fmt) {
[...]
case 'P':
/* Always print an explicitly whitelisted kernel pointer. */
break;
case 'K':
/* Always print %pK values when there are no restrictions. */
if (!kptr_restrict)
break;
/* Cleanse %pK values for non-privileged users. */
if (kptr_restrict == 1) {
const struct cred *cred;
/*
* kptr_restrict==1 cannot be used in IRQ context
* because its test for CAP_SYSLOG would be meaningless.
*/
if (in_irq() || in_serving_softirq() || in_nmi()) {
if (spec.field_width == -1)
spec.field_width = default_width;
return string(buf, end, "pK-error", spec);
}
/*
* Only print the real pointer value if the current
* process has CAP_SYSLOG and is running with the
* same credentials it started with. This is because
* access to files is checked at open() time, but %p
* checks permission at read() time. We don't want to
* leak pointer values if a binary opens a file using
* %pK and then elevates privileges before reading it.
*/
cred = current_cred();
if (!has_capability_noaudit(current, CAP_SYSLOG) ||
!uid_eq(cred->euid, cred->uid) ||
!gid_eq(cred->egid, cred->gid))
ptr = NULL;
break;
}
/* Always cleanse %pK values in higher restrition levels. */
ptr = NULL;
break;
default:
/*
* Also plain pointers (%p) are always cleansed in higher
* restriction levels.
*/
if (kptr_restrict >= 3)
ptr = NULL;
}Best Regards,
Petr
Greg KH
maintainerRe: [RFC 1/6] lib: vsprintf: additional kernel pointer filtering options
Quoted 31 lines · Petr Mladek
Quoted 16 lines · Greg KH
From: Dave Weinstein <[email protected]>
Add the
kptr_restrictsetting of 3 which results in both %p and %pK values being replaced by zeros.Add an additional %pP value inspired by the Grsecurity option which explicitly whitelists pointers for output.
This patch is based on work by William Roberts
<[email protected]>lib/vsprintf.c +0
-0lib/vsprintf.c struct printf_spec { · line 403
Quoted 9 lines
lib/vsprintf.c +6
-0@@ -395,6 +395,16 @@ struct printf_spec {#define FIELD_WIDTH_MAX ((1 << 23) - 1)#define PRECISION_MAX ((1 << 15) - 1) +int kptr_restrict__read_mostly ; + +/* +* Always cleanse %p and %pK specifiers +*/ +static inline int kptr_restrict_always_cleanse_pointers (void )The name of the function is very long and still confusing. It uses the word "always" but there are many types of pointers that are not cleared with this condition, for example %pP, %pa.
Quoted 2 lines
Do we need this helper function at all? It is used a weird way, see below.
Thanks for the comments, I'll revise this for the next version, thanks so much for the review, much appreciated.
thanks,
greg k-h
Greg KH
maintainerRFC 2/6 lib: vsprintf: whitelist stack traces
In reply to Greg KHFrom: Dave Weinstein <[email protected]>
Use the %pP functionality to explicitly allow kernel pointers to be logged for stack traces
--- arch/arm64/kernel/traps.c | 4 ++-- include/linux/kallsyms.h | 2 +- kernel/printk/printk.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-)
arch/arm64/kernel/traps.c +2 -2
@@ -146,7 +146,7 @@ static void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk)unsigned long irq_stack_ptr;int skip; -pr_debug ("%s(regs = %p tsk = %p)\n" , __func__, regs, tsk); +pr_debug ("%s(regs = %pP tsk = %pP)\n" , __func__, regs, tsk);if (!tsk) tsk = current;
@@ -252,7 +252,7 @@ static int __die(const char *str, int err, struct pt_regs *regs)print_modules ();__show_regs (regs); -pr_emerg ("Process %.*s (pid: %d, stack limit = 0x%p)\n" , +pr_emerg ("Process %.*s (pid: %d, stack limit = 0x%pP)\n" ,TASK_COMM_LEN , tsk->comm,task_pid_nr (tsk),end_of_stack (tsk));
include/linux/kallsyms.h +1 -1
@@ -122,7 +122,7 @@ static inline void print_symbol(const char *fmt, unsigned long addr)static inline void print_ip_sym (unsigned long ip) { -printk ("[<%p>] %pS\n" , (void *) ip, (void *) ip); +printk ("[<%pP>] %pS\n" , (void *) ip, (void *) ip); }#endif /*_LINUX_KALLSYMS_H*/
kernel/printk/printk.c +1 -1
@@ -3110,7 +3110,7 @@ void show_regs_print_info(const char *log_lvl) {dump_stack_print_info (log_lvl); -printk ("%stask: %p task.stack: %p\n" , +printk ("%stask: %pP task.stack: %pP\n" , log_lvl, current,task_stack_page (current)); }
Signature · 1 line
2.12.2
Greg KH
maintainerRFC 3/6 lib: vsprintf: physical address kernel pointer filtering options
In reply to Greg KHFrom: Dave Weinstein <[email protected]>
Add the kptr_restrict setting of 4 which results in %pa and %p[rR] values being replaced by zeros.
--- Documentation/sysctl/kernel.txt | 8 +++++++- kernel/sysctl.c | 3 +-- lib/vsprintf.c | 33 ++++++++++++++++++++++++++++++--- 3 files changed, 38 insertions(+), 6 deletions(-)
Documentation/sysctl/kernel.txt +7 -1
@@ -393,7 +393,13 @@ When kptr_restrict is set to (2), kernel pointers printed using %pK will be replaced with 0's regardless of privileges. When kptr_restrict is set to (3), kernel pointers printed using -%p and %pK will be replaced with 0's regardless of privileges. +%p and %pK will be replaced with 0's regardless of privileges, +however kernel pointers printed using %pP will continue to be printed. + +When kptr_restrict is set to (4), kernel pointers printed with +%p, %pK, %pa, and %p[rR] will be replaced with 0's regardless of +privileges. Kernel pointers printed using %pP will continue to be +printed. ==============================================================
kernel/sysctl.c +1 -2
@@ -129,7 +129,6 @@ static unsigned long one_ul = 1;static int one_hundred =100 ;static int one_thousand =1000 ;#ifdef CONFIG_PRINTK -static int three =3 ;static int ten_thousand =10000 ;#endif #ifdef CONFIG_PERF_EVENTS
@@ -831,7 +830,7 @@ static struct ctl_table kern_table[] = { .mode = 0644, .proc_handler = proc_dointvec_minmax_sysadmin, .extra1 = &zero, - .extra2 = &three, + .extra2 = &four, },#endif {
lib/vsprintf.c +30 -3
@@ -405,6 +405,22 @@ static inline int kptr_restrict_always_cleanse_pointers(void)return kptr_restrict >=3 ; } +/* +* Always cleanse physical addresses (%pa* specifiers) +*/ +static inline int kptr_restrict_cleanse_addresses (void ) +{ +return kptr_restrict >=4 ; +} + +/* +* Always cleanse resource addresses (%p[rR] specifiers) +*/ +static inline int kptr_restrict_cleanse_resources (void ) +{ +return kptr_restrict >=4 ; +} +static noinline_for_stack char *number(char *buf, char *end, unsigned long long num,struct printf_spec spec)
@@ -757,6 +773,7 @@ char *resource_string(char *buf, char *end, struct resource *res,char *p = sym, *pend = sym +sizeof (sym);int decode = (fmt[0 ] =='R' ) ?1 :0 ; +int cleanse =kptr_restrict_cleanse_resources ();const struct printf_spec *specp; *p++ ='[' ;
@@ -784,10 +801,11 @@ char *resource_string(char *buf, char *end, struct resource *res, p =string (p, pend,"size " , str_spec); p =number (p, pend,resource_size (res), *specp); }else { - p =number (p, pend, res->start, *specp); + p =number (p, pend, cleanse ?0UL : res->start, *specp);if (res->start != res->end) { *p++ ='-' ; - p =number (p, pend, res->end, *specp); + p =number (p, pend, cleanse ? + res->end - res->start : res->end, *specp); } }if (decode) {
@@ -1390,7 +1408,9 @@ char *address_val(char *buf, char *end, const void *addr, const char *fmt)break ; } -return special_hex_number (buf, end, num, size); +return special_hex_number (buf, end, +kptr_restrict_cleanse_addresses () ?0UL : num, + size); } static noinline_for_stack
@@ -1581,6 +1601,12 @@ char *flags_string(char *buf, char *end, void *flags_ptr, const char *fmt)* * Note: That for kptr_restrict set to 3, %p and %pK have the same * meaning. +* +* Note: That for kptr_restrict set to 4, %pa will null out the physical +* address. +* +* Note: That for kptr_restrict set to 4, %p[rR] will null out the memory +* address. */ static noinline_for_stack char *pointer(const char *fmt, char *buf, char *end, void *ptr,
@@ -1738,6 +1764,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr, }case 2 :/* restrict only %pK */ case 3 :/* restrict all non-extensioned %p and %pK */ +case 4 :/* restrict all non-extensioned %p, %pK, %pa*, %p[rR] */ default : ptr =NULL ;break ;
Signature · 1 line
2.12.2
Re: [kernel-hardening] [RFC 3/6] lib: vsprintf: physical address kernel pointer filtering options
Quoted 4 lines · Greg KH
From: Dave Weinstein <[email protected]>
Add the
kptr_restrictsetting of 4 which results in %pa and %p[rR] values being replaced by zeros.
Given that '%pa' is:
* - 'a[pd]' For address types [p] phys_addr_t, [d] dma_addr_t and derivatives * (default assumed to be phys_addr_t, passed by reference)
what is the thread model which hiding physical addresses from attackers protects against? I can see why virtual addresses would be obviously dangerous but physical addresses seem less obvious and I didn't see it spelled out in any of the commit messages or added comments in the thread.
I think a comment somewhere would be useful for people who are trying to decide if they should use %pa vs %paP etc.
Ian.
Greg KH
maintainerRFC 4/6 lib: vsprintf: default kptr_restrict to the maximum value
In reply to Greg KHFrom: Dave Weinstein <[email protected]>
Set the initial value of kptr_restrict to the maximum setting rather than the minimum setting, to ensure that early boot logging is not leaking information.
--- lib/vsprintf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
lib/vsprintf.c +1 -1
@@ -395,7 +395,7 @@ struct printf_spec {#define FIELD_WIDTH_MAX ((1 << 23) - 1)#define PRECISION_MAX ((1 << 15) - 1) -int kptr_restrict__read_mostly ; +int kptr_restrict__read_mostly =4 ;/* * Always cleanse %p and %pK specifiers
Signature · 1 line
2.12.2
Greg KH
maintainerRFC 5/6 lib: vsprintf: Add "%paP", "%padP" options
In reply to Greg KHFrom: Chris Fries <[email protected]>
Add %paP and %padP for physical address that need to always be shown regardless of kptr restrictions.
--- Documentation/printk-formats.txt | 10 ++++++---- lib/vsprintf.c | 12 +++++++++--- 2 files changed, 15 insertions(+), 7 deletions(-)
Documentation/printk-formats.txt +6 -4
@@ -82,18 +82,20 @@ Struct Resources: Physical addresses types phys_addr_t: - %pa[p] 0x01234567 or 0x0123456789abcdef + %pa[p][P] 0x01234567 or 0x0123456789abcdef For printing a phys_addr_t type (and its derivatives, such as resource_size_t) which can vary based on build options, regardless of - the width of the CPU data path. Passed by reference. + the width of the CPU data path. Passed by reference. Use the trailing + 'P' if it needs to be always shown. DMA addresses types dma_addr_t: - %pad 0x01234567 or 0x0123456789abcdef + %pad[P] 0x01234567 or 0x0123456789abcdef For printing a dma_addr_t type which can vary based on build options, - regardless of the width of the CPU data path. Passed by reference. + regardless of the width of the CPU data path. Passed by reference. Use + the trailing 'P' if it needs to be always shown. Raw buffer as an escaped string:
lib/vsprintf.c +9 -3
@@ -1394,23 +1394,29 @@ static noinline_for_stackchar *address_val (char *buf,char *end,const void *addr,const char *fmt) {unsigned long long num; +int cleanse =kptr_restrict_cleanse_addresses (); +int decleanse_idx =1 ;int size;switch (fmt[1 ]) {case 'd' : num = *(const dma_addr_t *)addr; size =sizeof (dma_addr_t); + decleanse_idx =2 ;break ;case 'p' : + decleanse_idx =2 ; +/* fall thru */ default : num = *(const phys_addr_t *)addr; size =sizeof (phys_addr_t);break ; } -return special_hex_number (buf, end, -kptr_restrict_cleanse_addresses () ?0UL : num, - size); +/* 'P' on the tail means don't restrict the pointer. */ + cleanse = cleanse && (fmt[decleanse_idx] !='P' ); + +return special_hex_number (buf, end, cleanse ?0UL : num, size); } static noinline_for_stack
Signature · 1 line
2.12.2
Re: [RFC 5/6] lib: vsprintf: Add "%paP", "%padP" options
Quoted 4 lines · Greg KH
From: Chris Fries <[email protected]>
Add %paP and %padP for physical address that need to always be shown regardless of kptr restrictions.
The commit message could be improved.
I had to look at the actual code to see if %papP was supported.
Documentation/printk-formats.txt
Quoted 1 line
Documentation/printk-formats.txt +0
-0
[]
Documentation/printk-formats.txt Struct Resources: · line 85
Quoted 6 lines
Documentation/printk-formats.txt +1
-1@@ -82,18 +82,20 @@ Struct Resources: Physical addresses types phys_addr_t: - %pa[p] 0x01234567 or 0x0123456789abcdef + %pa[p][P] 0x01234567 or 0x0123456789abcdef
Well, that's good.
lib/vsprintf.c
Quoted 1 line
lib/vsprintf.c +0
-0
[]
lib/vsprintf.c static noinline_for_stack · line 1397
Quoted 5 lines
lib/vsprintf.c +1
-0@@ -1394,23 +1394,29 @@ static noinline_for_stackchar *address_val (char *buf,char *end,const void *addr,const char *fmt) {unsigned long long num; +int cleanse =kptr_restrict_cleanse_addresses ();
bool
Greg KH
maintainerRe: [RFC 5/6] lib: vsprintf: Add "%paP", "%padP" options
Quoted 7 lines · Joe Perches
Quoted 4 lines · Greg KH
From: Chris Fries <[email protected]>
Add %paP and %padP for physical address that need to always be shown regardless of kptr restrictions.
The commit message could be improved.
Good point, I'll work on that, thanks.
Quoted 15 lines
I had to look at the actual code to see if %papP was supported.
Documentation/printk-formats.txt
Quoted 1 line
Documentation/printk-formats.txt +0
-0[]
Documentation/printk-formats.txt Struct Resources: · line 85
Quoted 6 lines
Documentation/printk-formats.txt +1
-1@@ -82,18 +82,20 @@ Struct Resources: Physical addresses types phys_addr_t: - %pa[p] 0x01234567 or 0x0123456789abcdef + %pa[p][P] 0x01234567 or 0x0123456789abcdefWell, that's good.
lib/vsprintf.c
Quoted 1 line
lib/vsprintf.c +0
-0[]
Quoted 7 lines
lib/vsprintf.c static noinline_for_stack · line 1397
Quoted 5 lines
lib/vsprintf.c +1
-0@@ -1394,23 +1394,29 @@ static noinline_for_stackchar *address_val (char *buf,char *end,const void *addr,const char *fmt) {unsigned long long num; +int cleanse =kptr_restrict_cleanse_addresses ();bool
I'll change this, thanks.
greg k-h
Petr Mladek
maintainerRe: [RFC 5/6] lib: vsprintf: Add "%paP", "%padP" options
In reply to Greg KHQuoted 5 lines · Greg KH
From: Chris Fries <[email protected]>
Add %paP and %padP for physical address that need to always be shown regardless of kptr restrictions.
Quoted 8 lines
Cc William Roberts <[email protected]>Cc Dave Weinstein <[email protected]>Signed-off-by Chris Fries <[email protected]>Signed-off-by Greg Kroah-Hartman <[email protected]>--- Documentation/printk-formats.txt | 10 ++++++---- lib/vsprintf.c | 12 +++++++++--- 2 files changed, 15 insertions(+), 7 deletions(-)
This patch should update also the section about kptr_restrict in Documentation/sysctl/kernel.txt. It should mention that the trailing P allows to see pointers also for %pa, and %p[rR] formats when the level is 4.
Best Regards,
Petr
Greg KH
maintainerRe: [RFC 5/6] lib: vsprintf: Add "%paP", "%padP" options
Quoted 15 lines · Petr Mladek
Quoted 5 lines · Greg KH
From: Chris Fries <[email protected]>
Add %paP and %padP for physical address that need to always be shown regardless of kptr restrictions.
Quoted 8 lines
Cc William Roberts <[email protected]>Cc Dave Weinstein <[email protected]>Signed-off-by Chris Fries <[email protected]>Signed-off-by Greg Kroah-Hartman <[email protected]>--- Documentation/printk-formats.txt | 10 ++++++---- lib/vsprintf.c | 12 +++++++++--- 2 files changed, 15 insertions(+), 7 deletions(-)
Quoted 5 lines
This patch should update also the section about
kptr_restrictinDocumentation/sysctl/kernel.txt. It should mention that the trailing P allows to see pointers also for %pa, and %p[rR] formats when the level is 4.
Ah, good point, will do!
greg k-h
Greg KH
maintainerRFC 6/6 drivers: uio: Un-restrict sysfs pointers for UIO
In reply to Greg KHFrom: Chris Fries <[email protected]>
The addr and size on the UIO devices are required by userspace to function properly. Let's unrestrict these by adding the 'P' modifier to %p and %pa.
--- drivers/uio/uio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/uio/uio.c +2 -2
@@ -56,12 +56,12 @@ static ssize_t map_name_show(struct uio_mem *mem, char *buf)static ssize_t map_addr_show (struct uio_mem *mem,char *buf) { -return sprintf (buf,"%pa\n" , &mem->addr); +return sprintf (buf,"%paP\n" , &mem->addr); }static ssize_t map_size_show (struct uio_mem *mem,char *buf) { -return sprintf (buf,"%pa\n" , &mem->size); +return sprintf (buf,"%paP\n" , &mem->size); }static ssize_t map_offset_show (struct uio_mem *mem,char *buf)
Signature · 1 line
2.12.2
Hello Greg,
Quoted 10 lines · Greg KH
Here's a short patch series from Chris Fries and Dave Weinstein that implement some new restrictions when printing out kernel pointers, as well as the ability to whitelist kernel pointers where needed.
These patches are based on work from William Roberts, and also is inspired by grsecurity's %pP to specifically whitelist a kernel pointer, where it is always needed, like the last patch in the series shows, in the UIO drivers (UIO requires that you know the address, it's a hardware address, nothing wrong with seeing that...)
Quoted 5 lines
I haven't done much to this patch series, only forward porting it from an older kernel release (4.4) and a few minor tweaks. It applies cleanly on top of 4.11 as well as Linus's current development tree (10502 patches into the 4.12-rc1 merge window). I'm posting it now for comments if anyone sees anything wrong with this approach
overall, I don't see anything wrong.
Quoted 1 line
or thinks the things that are being whitelisted should not be?
can't say for sure, sorry.
-ss
Quoted 33 lines
-----Original Message-----
From: Sergey Senozhatsky [mailto:[email protected]]
Sent: Wednesday, May 10, 2017 6:38 PM
To: Greg KH <[email protected]>
Cc: [email protected]; Petr Mladek <[email protected]>; Sergey Senozhatsky <[email protected]>; linux- [email protected]; Catalin Marinas <[email protected]>; Will Deacon <[email protected]>; Steven Rostedt <[email protected]>; Roberts, William C <[email protected]>; Chris Fries <[email protected]>; Dave Weinstein <[email protected]>
Subject: Re: [RFC 00/06] printk: add more new kernel pointer filter options.Hello Greg,
Quoted 10 lines · Greg KH
Here's a short patch series from Chris Fries and Dave Weinstein that implement some new restrictions when printing out kernel pointers, as well as the ability to whitelist kernel pointers where needed.
These patches are based on work from William Roberts, and also is inspired by grsecurity's %pP to specifically whitelist a kernel pointer, where it is always needed, like the last patch in the series shows, in the UIO drivers (UIO requires that you know the address, it's a hardware address, nothing wrong with seeing that...)
Quoted 5 lines
I haven't done much to this patch series, only forward porting it from an older kernel release (4.4) and a few minor tweaks. It applies cleanly on top of 4.11 as well as Linus's current development tree (10502 patches into the 4.12-rc1 merge window). I'm posting it now for comments if anyone sees anything wrong with this approach
Quoted 7 lines
overall, I don't see anything wrong.
Quoted 1 line
or thinks the things that are being whitelisted should not be?
can't say for sure, sorry.
-ss
I almost missed this, none of the mail was delivered to my inbox...
Anyways, I am glad to see this revived and I don't have any Comments besides thanks.
Greg KH
maintainerQuoted 35 lines · William C
Quoted 35 lines
-----Original Message-----
From: Sergey Senozhatsky [mailto:[email protected]]
Sent: Wednesday, May 10, 2017 6:38 PM
To: Greg KH <[email protected]>
Cc: [email protected]; Petr Mladek <[email protected]>; Sergey Senozhatsky <[email protected]>; linux- [email protected]; Catalin Marinas <[email protected]>; Will Deacon <[email protected]>; Steven Rostedt <[email protected]>; Roberts, William C <[email protected]>; Chris Fries <[email protected]>; Dave Weinstein <[email protected]>
Subject: Re: [RFC 00/06] printk: add more new kernel pointer filter options.Hello Greg,
Quoted 10 lines · Greg KH
Here's a short patch series from Chris Fries and Dave Weinstein that implement some new restrictions when printing out kernel pointers, as well as the ability to whitelist kernel pointers where needed.
These patches are based on work from William Roberts, and also is inspired by grsecurity's %pP to specifically whitelist a kernel pointer, where it is always needed, like the last patch in the series shows, in the UIO drivers (UIO requires that you know the address, it's a hardware address, nothing wrong with seeing that...)
Quoted 5 lines
I haven't done much to this patch series, only forward porting it from an older kernel release (4.4) and a few minor tweaks. It applies cleanly on top of 4.11 as well as Linus's current development tree (10502 patches into the 4.12-rc1 merge window). I'm posting it now for comments if anyone sees anything wrong with this approach
overall, I don't see anything wrong.
Quoted 7 lines
Quoted 5 lines
Quoted 1 line
or thinks the things that are being whitelisted should not be?
can't say for sure, sorry.
-ssI almost missed this, none of the mail was delivered to my inbox...
Why not? Did I get the address wrong?
Quoted 2 lines
Anyways, I am glad to see this revived and I don't have any Comments besides thanks.
Acks for the patches are always appreciated :)
I'll revise this in the next few weeks and send out a new series.
thanks,
greg k-h
Quoted 55 lines
-----Original Message-----
From: Greg KH [mailto:[email protected]]
Sent: Thursday, May 18, 2017 7:13 AM
To: Roberts, William C <[email protected]>
Cc: Sergey Senozhatsky <[email protected]>; kernel- [email protected]; Petr Mladek <[email protected]>; Sergey Senozhatsky <[email protected]>; [email protected]; Catalin Marinas <[email protected]>; Will Deacon <[email protected]>; Steven Rostedt <[email protected]>; Chris Fries <[email protected]>; Dave Weinstein <[email protected]>
Subject: Re: [RFC 00/06] printk: add more new kernel pointer filter options.Quoted 40 lines · William C
Quoted 40 lines
-----Original Message-----
From: Sergey Senozhatsky [mailto:[email protected]]
Sent: Wednesday, May 10, 2017 6:38 PM
To: Greg KH <[email protected]>
Cc: [email protected]; Petr Mladek <[email protected]>; Sergey Senozhatsky <[email protected]>; linux- [email protected]; Catalin Marinas <[email protected]>; Will Deacon <[email protected]>; Steven Rostedt <[email protected]>; Roberts, William C <[email protected]>; Chris Fries <[email protected]>; Dave Weinstein <[email protected]>
Subject: Re: [RFC 00/06] printk: add more new kernel pointer filter options.Hello Greg,
Quoted 11 lines · Greg KH
Here's a short patch series from Chris Fries and Dave Weinstein that implement some new restrictions when printing out kernel pointers, as well as the ability to whitelist kernel pointers where needed.
These patches are based on work from William Roberts, and also is inspired by grsecurity's %pP to specifically whitelist a kernel pointer, where it is always needed, like the last patch in the series shows, in the UIO drivers (UIO requires that you know the address, it's a hardware address, nothing wrong with seeing that...)
Quoted 6 lines
I haven't done much to this patch series, only forward porting it from an older kernel release (4.4) and a few minor tweaks. It applies cleanly on top of 4.11 as well as Linus's current development tree
(10502 patches into the 4.12-rc1 merge window). I'm posting it now for comments if anyone sees anything wrong with this approachoverall, I don't see anything wrong.
Quoted 1 line
or thinks the things that are being whitelisted should not be?
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
can't say for sure, sorry.
-ssI almost missed this, none of the mail was delivered to my inbox...
Why not? Did I get the address wrong?
I don't think so. I've had weird issues with my Intel email address and mailing lists before. On the selinux mailing list they kept getting bounces when sending Me email, but it's only that list. I'm just going to blame it on something within our corporate network.
Quoted 7 lines
Quoted 3 lines
Anyways, I am glad to see this revived and I don't have any Comments besides thanks.
Acks for the patches are always appreciated :)
I'll revise this in the next few weeks and send out a new series.
I see some comments on clarifying the docs that seem spot on. I'll look at the next series and I will test them, if they look good to me, I'll ack away :-)
Quoted 4 lines
thanks,
greg k-h