Re: [patch 1/1] net: convert %p usage to %pK

[Eric Dumazet <eric.dumazet@gmail.com>]

Le mardi 24 mai 2011 à 08:57 +0200, Ingo Molnar a écrit :
> * Joe Perches <joe@perches.com> wrote:
> 
> > 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 <eric.dumazet@gmail.com>
---
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;