[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1306220681.2638.40.camel@edumazet-laptop>
Date: Tue, 24 May 2011 09:04:41 +0200
From: Eric Dumazet <eric.dumazet@...il.com>
To: Ingo Molnar <mingo@...e.hu>
Cc: Joe Perches <joe@...ches.com>, David Miller <davem@...emloft.net>,
akpm@...ux-foundation.org, netdev@...r.kernel.org,
drosenberg@...curity.com, a.p.zijlstra@...llo.nl,
eparis@...isplace.org, eugeneteo@...nel.org, jmorris@...ei.org,
kees.cook@...onical.com, tgraf@...radead.org
Subject: Re: [patch 1/1] net: convert %p usage to %pK
Le mardi 24 mai 2011 à 08:57 +0200, Ingo Molnar a écrit :
> * Joe Perches <joe@...ches.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@...il.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;
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists