Re: [patch 1/1] net: convert %p usage to %pK
From: Eric Dumazet <hidden>
Date: 2011-05-24 07:04:46
Subsystem:
library code, networking [general], printk, the rest, vsprintf · Maintainers:
Andrew Morton, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Petr Mladek, Linus Torvalds, Steven Rostedt
Le mardi 24 mai 2011 à 08:57 +0200, Ingo Molnar a écrit :
* Joe Perches [off-list ref] wrote:quoted
Maybe for clarity it'd be better to use a switch/case or something like: if (kptr_restrict == 0) return ptr; if (ptr_restrict == 1 && has_capability_noaudit(current, CAP_SYSLOG)) return ptr; return NULL;While not breaking that line really - so something like: if (kptr_restrict == 0) return ptr; if (kptr_restrict == 1 && has_capability_noaudit(current, CAP_SYSLOG)) return ptr; return NULL; Thanks, Ingo
Here we are, thanks. [PATCH v2] inet_diag: hide socket pointers Provide a mayber_hide_ptr() helper and use it in inet_diag to not disclose kernel pointers to user, with following kptr_restrict logic : kptr_restrict = 0 : kernel pointers are not mangled kptr_restrict = 1 : if the current user does not have CAP_SYSLOG, kernel pointers are replaced by 0 kptr_restrict = 2 : kernel pointers are replaced by 0 Signed-off-by: Eric Dumazet <redacted> --- v2: kerneldoc and cleanup (Joe & Ingo feedback) include/linux/printk.h | 1 + lib/vsprintf.c | 25 +++++++++++++++++++++---- net/ipv4/inet_diag.c | 6 ++++-- 3 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/include/linux/printk.h b/include/linux/printk.h
index ee048e7..47c0cef 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h@@ -111,6 +111,7 @@ extern bool printk_timed_ratelimit(unsigned long *caller_jiffies, extern int printk_delay_msec; extern int dmesg_restrict; extern int kptr_restrict; +void *maybe_hide_ptr(void *ptr); void log_buf_kexec_setup(void); #else
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 1d659d7..db1a193 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c@@ -798,6 +798,26 @@ char *uuid_string(char *buf, char *end, const u8 *addr, } int kptr_restrict __read_mostly; +/** + * maybe_hide_ptr - Eventually nullify a kernel pointer given to user + * @ptr: kernel pointer + * + * kptr_restrict = 0 : kernel pointer not changed + * kptr_restrict = 1 : if the current user does not have CAP_SYSLOG, + * kernel pointer is replaced by 0 + * kptr_restrict = 2 : kernel pointer is replaced by 0 for all users. + */ +void *maybe_hide_ptr(void *ptr) +{ + if (!kptr_restrict) + return ptr; + + if (kptr_restrict == 1 && has_capability_noaudit(current, CAP_SYSLOG)) + return ptr; + + return NULL; +} +EXPORT_SYMBOL(maybe_hide_ptr); /* * Show a '%p' thing. A kernel extension is that the '%p' is followed
@@ -911,10 +931,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr, spec.field_width = 2 * sizeof(void *); return string(buf, end, "pK-error", spec); } - if (!((kptr_restrict == 0) || - (kptr_restrict == 1 && - has_capability_noaudit(current, CAP_SYSLOG)))) - ptr = NULL; + ptr = maybe_hide_ptr(ptr); break; } spec.flags |= SMALL;
diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index 6ffe94c..b5646a3 100644
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c@@ -84,6 +84,7 @@ static int inet_csk_diag_fill(struct sock *sk, struct inet_diag_meminfo *minfo = NULL; unsigned char *b = skb_tail_pointer(skb); const struct inet_diag_handler *handler; + u64 ptr; handler = inet_diag_table[unlh->nlmsg_type]; BUG_ON(handler == NULL);
@@ -114,8 +115,9 @@ static int inet_csk_diag_fill(struct sock *sk, r->idiag_retrans = 0; r->id.idiag_if = sk->sk_bound_dev_if; - r->id.idiag_cookie[0] = (u32)(unsigned long)sk; - r->id.idiag_cookie[1] = (u32)(((unsigned long)sk >> 31) >> 1); + ptr = (u64)maybe_hide_ptr(sk); + r->id.idiag_cookie[0] = (u32)ptr; + r->id.idiag_cookie[1] = (u32)(ptr >> 32); r->id.idiag_sport = inet->inet_sport; r->id.idiag_dport = inet->inet_dport;