lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <4CD8FDB5.6060905@hartkopp.net> Date: Tue, 09 Nov 2010 08:52:21 +0100 From: Oliver Hartkopp <socketcan@...tkopp.net> To: David Miller <davem@...emloft.net> CC: Urs Thuermann <urs@...ogud.escape.de>, netdev@...r.kernel.org, Dan Rosenberg <drosenberg@...curity.com>, security@...nel.org, Linus Torvalds <torvalds@...ux-foundation.org> Subject: Re: [PATCH] Fix CAN info leak/minor heap overflow On 05.11.2010 19:33, Urs Thuermann wrote: > This patch removes the leakage of kernel space addresses to userspace. > Instead, socket inode numbers are used to create unique proc file > names for CAN_BCM sockets and for referring to sockets in filter > lists. In addition, this makes debugging easier, since inode numbers > are also shown in ls -l /proc/<pid>/fd/<fd> and lsof(8) output. > > BTW, if kernel space addresses are considered security critical > information one should also take a look and possibly change > > /proc/net/{tcp,tcp6,udp,udp6,raw,raw6,unix} > > and maybe some others. > > The change of the procfs content leads to a new version string > 20101105. > > Signed-off-by: Urs Thuermann <urs@...ogud.escape.de> > Signed-off-by: Oliver Hartkopp <socketcan@...tkopp.net> Besides the ongoing(?) discussion about the exposed kernel addresses in procfs - what are your plans about this patch that already moves the kernel addresses to inode numbers? Is it something for net-2.6 / net-next-2.6 / stable ? Especially in this case we do not see any problems with userspace tools that could break as it would be for some other /proc/net entries. Once this patch is applied (and the procfs layout is changed anyway), i'd also like to send a patch from my backlog that would extend the procfs output for can-bcm with an additional drop counter. Best regards, Oliver > CC: Dan Rosenberg <drosenberg@...curity.com> > CC: Linus Torvalds <torvalds@...ux-foundation.org> > > --- > > diff --git a/include/linux/can/core.h b/include/linux/can/core.h > index 6c507be..e20a841 100644 > --- a/include/linux/can/core.h > +++ b/include/linux/can/core.h > @@ -19,7 +19,7 @@ > #include <linux/skbuff.h> > #include <linux/netdevice.h> > > -#define CAN_VERSION "20090105" > +#define CAN_VERSION "20101105" > > /* increment this number each time you change some user-space interface */ > #define CAN_ABI_VERSION "8" > diff --git a/net/can/bcm.c b/net/can/bcm.c > index 08ffe9e..0e81e04 100644 > --- a/net/can/bcm.c > +++ b/net/can/bcm.c > @@ -86,6 +86,12 @@ MODULE_LICENSE("Dual BSD/GPL"); > MODULE_AUTHOR("Oliver Hartkopp <oliver.hartkopp@...kswagen.de>"); > MODULE_ALIAS("can-proto-2"); > > +/* > + * Point to the sockets inode number inside the bcm ident string. > + * We skip the string length of "bcm " (== 4) created in bcm_init(). > + */ > +#define INODENUM(bo) (bo->ident + 4) > + > /* easy access to can_frame payload */ > static inline u64 GET_U64(const struct can_frame *cp) > { > @@ -125,7 +131,7 @@ struct bcm_sock { > struct list_head tx_ops; > unsigned long dropped_usr_msgs; > struct proc_dir_entry *bcm_proc_read; > - char procname [9]; /* pointer printed in ASCII with \0 */ > + char ident[32]; > }; > > static inline struct bcm_sock *bcm_sk(const struct sock *sk) > @@ -165,9 +171,7 @@ static int bcm_proc_show(struct seq_file *m, void *v) > struct bcm_sock *bo = bcm_sk(sk); > struct bcm_op *op; > > - seq_printf(m, ">>> socket %p", sk->sk_socket); > - seq_printf(m, " / sk %p", sk); > - seq_printf(m, " / bo %p", bo); > + seq_printf(m, ">>> socket inode %s", INODENUM(bo)); > seq_printf(m, " / dropped %lu", bo->dropped_usr_msgs); > seq_printf(m, " / bound %s", bcm_proc_getifname(ifname, bo->ifindex)); > seq_printf(m, " <<<\n"); > @@ -1168,7 +1172,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, > err = can_rx_register(dev, op->can_id, > REGMASK(op->can_id), > bcm_rx_handler, op, > - "bcm"); > + bo->ident); > > op->rx_reg_dev = dev; > dev_put(dev); > @@ -1177,7 +1181,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg, > } else > err = can_rx_register(NULL, op->can_id, > REGMASK(op->can_id), > - bcm_rx_handler, op, "bcm"); > + bcm_rx_handler, op, bo->ident); > if (err) { > /* this bcm rx op is broken -> remove it */ > list_del(&op->list); > @@ -1402,6 +1406,8 @@ static int bcm_init(struct sock *sk) > { > struct bcm_sock *bo = bcm_sk(sk); > > + snprintf(bo->ident, sizeof(bo->ident), "bcm %lu", sock_i_ino(sk)); > + > bo->bound = 0; > bo->ifindex = 0; > bo->dropped_usr_msgs = 0; > @@ -1466,7 +1472,7 @@ static int bcm_release(struct socket *sock) > > /* remove procfs entry */ > if (proc_dir && bo->bcm_proc_read) > - remove_proc_entry(bo->procname, proc_dir); > + remove_proc_entry(INODENUM(bo), proc_dir); > > /* remove device reference */ > if (bo->bound) { > @@ -1519,13 +1525,11 @@ static int bcm_connect(struct socket *sock, struct sockaddr *uaddr, int len, > > bo->bound = 1; > > - if (proc_dir) { > - /* unique socket address as filename */ > - sprintf(bo->procname, "%p", sock); > - bo->bcm_proc_read = proc_create_data(bo->procname, 0644, > + /* use unique socket inode number as filename */ > + if (proc_dir) > + bo->bcm_proc_read = proc_create_data(INODENUM(bo), 0644, > proc_dir, > &bcm_proc_fops, sk); > - } > > return 0; > } > diff --git a/net/can/proc.c b/net/can/proc.c > index f4265cc..15bed1c 100644 > --- a/net/can/proc.c > +++ b/net/can/proc.c > @@ -204,23 +204,17 @@ static void can_print_rcvlist(struct seq_file *m, struct hlist_head *rx_list, > > hlist_for_each_entry_rcu(r, n, rx_list, list) { > char *fmt = (r->can_id & CAN_EFF_FLAG)? > - " %-5s %08X %08x %08x %08x %8ld %s\n" : > - " %-5s %03X %08x %08lx %08lx %8ld %s\n"; > + " %-5s %08X %08x %8ld %s\n" : > + " %-5s %03X %08x %8ld %s\n"; > > seq_printf(m, fmt, DNAME(dev), r->can_id, r->mask, > - (unsigned long)r->func, (unsigned long)r->data, > r->matches, r->ident); > } > } > > static void can_print_recv_banner(struct seq_file *m) > { > - /* > - * can1. 00000000 00000000 00000000 > - * ....... 0 tp20 > - */ > - seq_puts(m, " device can_id can_mask function" > - " userdata matches ident\n"); > + seq_puts(m, " device can_id can_mask matches ident\n"); > } > > static int can_stats_proc_show(struct seq_file *m, void *v) > diff --git a/net/can/raw.c b/net/can/raw.c > index e88f610..e057f0d 100644 > --- a/net/can/raw.c > +++ b/net/can/raw.c > @@ -88,6 +88,7 @@ struct raw_sock { > struct can_filter dfilter; /* default/single filter */ > struct can_filter *filter; /* pointer to filter(s) */ > can_err_mask_t err_mask; > + char ident[32]; > }; > > /* > @@ -154,13 +155,14 @@ static void raw_rcv(struct sk_buff *oskb, void *data) > static int raw_enable_filters(struct net_device *dev, struct sock *sk, > struct can_filter *filter, int count) > { > + struct raw_sock *ro = raw_sk(sk); > int err = 0; > int i; > > for (i = 0; i < count; i++) { > err = can_rx_register(dev, filter[i].can_id, > filter[i].can_mask, > - raw_rcv, sk, "raw"); > + raw_rcv, sk, ro->ident); > if (err) { > /* clean up successfully registered filters */ > while (--i >= 0) > @@ -177,11 +179,12 @@ static int raw_enable_filters(struct net_device *dev, struct sock *sk, > static int raw_enable_errfilter(struct net_device *dev, struct sock *sk, > can_err_mask_t err_mask) > { > + struct raw_sock *ro = raw_sk(sk); > int err = 0; > > if (err_mask) > err = can_rx_register(dev, 0, err_mask | CAN_ERR_FLAG, > - raw_rcv, sk, "raw"); > + raw_rcv, sk, ro->ident); > > return err; > } > @@ -281,6 +284,8 @@ static int raw_init(struct sock *sk) > { > struct raw_sock *ro = raw_sk(sk); > > + snprintf(ro->ident, sizeof(ro->ident), "raw %lu", sock_i_ino(sk)); > + > ro->bound = 0; > ro->ifindex = 0; > -- 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