Fix leaking of kernel heap addresses in net/
Subsystems (5)
PATCH 3/10 Fix leaking of kernel heap addresses in net/
net/ipv4/raw.c +20 -7
@@ -948,13 +948,26 @@ static void raw_sock_seq_show(struct seq_file *seq, struct sock *sp, int i)__u16 destp =0 , srcp = inet->inet_num; -seq_printf (seq,"%4d: %08X:%04X %08X:%04X" -" %02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %p %d\n" , - i, src, srcp, dest, destp, sp->sk_state, -sk_wmem_alloc_get (sp), -sk_rmem_alloc_get (sp), -0 ,0L ,0 ,sock_i_uid (sp),0 ,sock_i_ino (sp), -atomic_read (&sp->sk_refcnt), sp,atomic_read (&sp->sk_drops)); +/* Only expose kernel addresses to privileged readers */ +if (capable (CAP_NET_ADMIN )) +seq_printf (seq,"%4d: %08X:%04X %08X:%04X" +" %02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %p %d\n" , + i, src, srcp, dest, destp, sp->sk_state, +sk_wmem_alloc_get (sp), +sk_rmem_alloc_get (sp), +0 ,0L ,0 ,sock_i_uid (sp),0 ,sock_i_ino (sp), +atomic_read (&sp->sk_refcnt), + sp,atomic_read (&sp->sk_drops)); +else +seq_printf (seq,"%4d: %08X:%04X %08X:%04X" +" %02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %d %d\n" , + i, src, srcp, dest, destp, sp->sk_state, +sk_wmem_alloc_get (sp), +sk_rmem_alloc_get (sp), +0 ,0L ,0 ,sock_i_uid (sp),0 ,sock_i_ino (sp), +atomic_read (&sp->sk_refcnt), +0 ,atomic_read (&sp->sk_drops)); + }static int raw_seq_show (struct seq_file *seq,void *v)
net/ipv4/tcp_ipv4.c +95 -41
154 lines not on this page · raw message
net/ipv4/udp.c +20 -8
@@ -2045,14 +2045,26 @@ static void udp4_format_sock(struct sock *sp, struct seq_file *f,__u16 destp =ntohs (inet->inet_dport);__u16 srcp =ntohs (inet->inet_sport); -seq_printf (f,"%5d: %08X:%04X %08X:%04X" -" %02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %p %d%n" , - bucket, src, srcp, dest, destp, sp->sk_state, -sk_wmem_alloc_get (sp), -sk_rmem_alloc_get (sp), -0 ,0L ,0 ,sock_i_uid (sp),0 ,sock_i_ino (sp), -atomic_read (&sp->sk_refcnt), sp, -atomic_read (&sp->sk_drops), len); +/* Only expose kernel addresses to privileged readers */ +if (capable (CAP_NET_ADMIN )) +seq_printf (f,"%5d: %08X:%04X %08X:%04X" +" %02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %p %d%n" , + bucket, src, srcp, dest, destp, sp->sk_state, +sk_wmem_alloc_get (sp), +sk_rmem_alloc_get (sp), +0 ,0L ,0 ,sock_i_uid (sp),0 ,sock_i_ino (sp), +atomic_read (&sp->sk_refcnt), sp, +atomic_read (&sp->sk_drops), len); +else +seq_printf (f,"%5d: %08X:%04X %08X:%04X" +" %02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %d %d%n" , + bucket, src, srcp, dest, destp, sp->sk_state, +sk_wmem_alloc_get (sp), +sk_rmem_alloc_get (sp), +0 ,0L ,0 ,sock_i_uid (sp),0 ,sock_i_ino (sp), +atomic_read (&sp->sk_refcnt),0 , +atomic_read (&sp->sk_drops), len); + }int udp4_seq_show (struct seq_file *seq,void *v)
Quoted 24 lines · Dan Rosenberg
net/ipv4/raw.c +9
-7@@ -948,13 +948,26 @@ static void raw_sock_seq_show(struct seq_file *seq, struct sock *sp, int i)__u16 destp =0 , srcp = inet->inet_num; -seq_printf (seq,"%4d: %08X:%04X %08X:%04X" -" %02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %p %d\n" , - i, src, srcp, dest, destp, sp->sk_state, -sk_wmem_alloc_get (sp), -sk_rmem_alloc_get (sp), -0 ,0L ,0 ,sock_i_uid (sp),0 ,sock_i_ino (sp), -atomic_read (&sp->sk_refcnt), sp,atomic_read (&sp->sk_drops)); +/* Only expose kernel addresses to privileged readers */ +if (capable (CAP_NET_ADMIN )) + seq_printf(seq, "%4d: %08X:%04X %08X:%04X" + " %02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %p %d\n", + i, src, srcp, dest, destp, sp->sk_state, + sk_wmem_alloc_get(sp), + sk_rmem_alloc_get(sp), + 0, 0L, 0, sock_i_uid(sp), 0, sock_i_ino(sp), + atomic_read(&sp->sk_refcnt),
net/ipv4/raw.c static void raw_sock_seq_show(struct seq_file *seq, struct sock *sp, int i) · line 969
Quoted 10 lines
net/ipv4/raw.c +10
-0+ sp, atomic_read(&sp->sk_drops)); + else + seq_printf(seq, "%4d: %08X:%04X %08X:%04X" +" %02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %d %d\n" , + i, src, srcp, dest, destp, sp->sk_state, +sk_wmem_alloc_get (sp), +sk_rmem_alloc_get (sp), +0 ,0L ,0 ,sock_i_uid (sp),0 ,sock_i_ino (sp), +atomic_read (&sp->sk_refcnt), + 0, atomic_read(&sp->sk_drops));
If we really have to do this. At least don't duplicate all this code. Do the check in the printf argument:
seq_printf(seq, "%4d: %08X:%04X %08X:%04X" ... capable(CAP_NET_ADMIN) ? sp : 0,
I would even move the decision whether to expose kernel addresses or not to a function so we can change behavior in one place.
Quoted 11 lines
If we really have to do this. At least don't duplicate all this code. Do the check in the printf argument:
seq_printf(seq, "%4d: %08X:%04X %08X:%04X" ... capable(CAP_NET_ADMIN) ? sp : 0,I would even move the decision whether to expose kernel addresses or not to a function so we can change behavior in one place.
I wrote it this way because the format specifier must also be changed, or the %p output will print "(null)", which cannot be parsed by userspace programs expecting "(nil)" or 0. I could include another check inside the format specifier, but that seemed pretty ugly. But then again, it's ugly either way.
-Dan
Quoted 7 lines · Dan Rosenberg
Quoted 7 lines
If we really have to do this. At least don't duplicate all this code. Do the check in the printf argument:
seq_printf(seq, "%4d: %08X:%04X %08X:%04X" ... capable(CAP_NET_ADMIN) ? sp : 0,
Quoted 8 lines
Quoted 2 lines
I would even move the decision whether to expose kernel addresses or not to a function so we can change behavior in one place.
I wrote it this way because the format specifier must also be changed, or the %p output will print "(null)", which cannot be parsed by userspace programs expecting "(nil)" or 0. I could include another check inside the format specifier, but that seemed pretty ugly. But then again, it's ugly either way.
Considering the amount of duplication you are about to do, you may want to think about adding a new pointer format extension then. We already have special '%p' modes for IPv6 addresse, MAC addresses and various other pointer types. It wouldn't be hard to add one which does not print (null).
Quoted 4 lines · Dan Rosenberg
net/ipv4/raw.c +0
-0
[...]
Quoted 9 lines
net/ipv4/raw.c +9
-0+/* Only expose kernel addresses to privileged readers */ +if (capable (CAP_NET_ADMIN )) + seq_printf(seq, "%4d: %08X:%04X %08X:%04X" + " %02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %p %d\n", + i, src, srcp, dest, destp, sp->sk_state, + sk_wmem_alloc_get(sp), + sk_rmem_alloc_get(sp), + 0, 0L, 0, sock_i_uid(sp), 0, sock_i_ino(sp), + atomic_read(&sp->sk_refcnt),
net/ipv4/raw.c static void raw_sock_seq_show(struct seq_file *seq, struct sock *sp, int i) · line 969
Quoted 10 lines
net/ipv4/raw.c +10
-0+ sp, atomic_read(&sp->sk_drops)); + else + seq_printf(seq, "%4d: %08X:%04X %08X:%04X" +" %02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %d %d\n" , + i, src, srcp, dest, destp, sp->sk_state, +sk_wmem_alloc_get (sp), +sk_rmem_alloc_get (sp), +0 ,0L ,0 ,sock_i_uid (sp),0 ,sock_i_ino (sp), +atomic_read (&sp->sk_refcnt), + 0, atomic_read(&sp->sk_drops));
[...]
This could just be written as:
seq_printf(seq, "%4d: %08X:%04X %08X:%04X" " %02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %lx %d\n", i, src, srcp, dest, destp, sp->sk_state, sk_wmem_alloc_get(sp), sk_rmem_alloc_get(sp), 0, 0L, 0, sock_i_uid(sp), 0, sock_i_ino(sp), atomic_read(&sp->sk_refcnt), capable(CAP_NET_ADMIN) ? (unsigned long)sp : 0UL, atomic_read(&sp->sk_drops));
Similarly for other formats that you want to make conditional on CAP_NET_ADMIN.
Ben.
Signature · 3 lines
Ben Hutchings, Senior Software Engineer, Solarflare Communications Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked.
Quoted 4 lines
Similarly for other formats that you want to make conditional on
CAP_NET_ADMIN.
I wanted to avoid a %p output of "(null)" in case userspace tools couldn't handle that, but I agree this is much nicer looking. I'm currently putting together a new format specifier instead, as per the suggestions of Eric and David.
-Dan