Re: LSF and SOCK_PACKET

Andi Kleen (ak@muc.de)
Thu, 15 Oct 1998 21:42:47 +0200


On Thu, Oct 15, 1998 at 08:32:23PM +0200, kuznet@ms2.inr.ac.ru wrote:
> Hello!
>
> > In short it should be commented out or moved to "CONFIG_VERY_EXPERIMENTAL"
> > for 2.2.0 and check CAP_NET_RAW ?
>
> It is too useful on packet socket to be put to experiamntal section.
> It should be on by default really.
>
> Actually, only the fact that it allows user to filter tcp segments
> and to mangle(!) lengths of tcp packets is enough to prohibit
> its not-privileged usage. At least, it apparently gives user
> nice ways to confuse our tcp stack.
>
> Diagnosis is CAP_NET_RAW.

This patch should cure the worst bugs (except in the filter checker,
which I didn't look at).

Fixed:
- Races at filter setup/detachment
- Length check could overflow
- Memory leak at socket close
- User memory was directly accessed, which can be easily used to crash
the kernel.
- Checks CAP_NET_RAW now.

-Andi

Index: linux/net/core/sock.c
===================================================================
RCS file: /vger/u4/cvs/linux/net/core/sock.c,v
retrieving revision 1.73
diff -u -r1.73 sock.c
--- sock.c 1998/10/03 16:08:10 1.73
+++ sock.c 1998/10/15 13:20:00
@@ -294,6 +294,10 @@
{
char devname[IFNAMSIZ];

+ /* Sorry... */
+ if (!capable(CAP_NET_RAW))
+ return -EPERM;
+
/* Bind this socket to a particular device like "eth0",
* as specified in the passed interface name. If the
* name is "" or the option length is zero the socket
@@ -327,7 +331,11 @@

#ifdef CONFIG_FILTER
case SO_ATTACH_FILTER:
- if(optlen < sizeof(struct sock_fprog))
+ /* Until the code is audited.. */
+ if (!capable(CAP_NET_RAW))
+ return -EPERM;
+
+ if(optlen != sizeof(struct sock_fprog))
return -EINVAL;

if(copy_from_user(&fprog, optval, sizeof(fprog)))
@@ -342,10 +350,14 @@
case SO_DETACH_FILTER:
if(sk->filter)
{
+ start_bh_atomic();
fprog.filter = sk->filter_data;
- kfree_s(fprog.filter, (sizeof(fprog.filter) * sk->filter));
sk->filter_data = NULL;
sk->filter = 0;
+ end_bh_atomic();
+
+ if (fprog.filter)
+ kfree(fprog.filter);
return 0;
}
else
@@ -808,6 +823,15 @@
struct sk_buff *skb;
if(list)
sklist_remove_socket(list, sk);
+
+#ifdef CONFIG_FILTER
+ if (sk->filter && sk->filter_data)
+ {
+ kfree(sk->filter_data);
+ sk->filter = 0;
+ sk->filter_data = NULL;
+ }
+#endif

while((skb=skb_dequeue(&sk->receive_queue))!=NULL)
{
Index: linux/net/core/filter.c
===================================================================
RCS file: /vger/u4/cvs/linux/net/core/filter.c,v
retrieving revision 1.1
diff -u -r1.1 filter.c
--- filter.c 1998/01/10 12:15:14 1.1
+++ filter.c 1998/10/15 13:20:01
@@ -11,6 +11,8 @@
* modify it under the terms of the GNU General Public License
* as published by the Free Software Foundation; either version
* 2 of the License, or (at your option) any later version.
+ *
+ * Andi Kleen - Fix a few bad bugs and races.
*/

#include <linux/config.h>
@@ -254,7 +256,6 @@
}
}

- printk(KERN_ERR "Filter ruleset ran off the end.\n");
return (0);
}

@@ -333,32 +334,37 @@
int sk_attach_filter(struct sock_fprog *fprog, struct sock *sk)
{
struct sock_filter *fp, *old_filter;
- int fsize = sizeof(struct sock_filter) * fprog->len;
+ unsigned int fsize = sizeof(struct sock_filter) * fprog->len;
int err;

/* Make sure new filter is there and in the right amounts. */
if(fprog->filter == NULL || fprog->len == 0 || fsize > BPF_MAXINSNS)
return (-EINVAL);

- if((err = sk_chk_filter(fprog->filter, fprog->len))==0)
- {
- /* If existing filter, remove it first */
- if(sk->filter)
- {
- old_filter = sk->filter_data;
- kfree_s(old_filter, (sizeof(old_filter) * sk->filter));
- sk->filter_data = NULL;
- }
-
- fp = (struct sock_filter *)kmalloc(fsize, GFP_KERNEL);
- if(fp == NULL)
- return (-ENOMEM);
+ fp = (struct sock_filter *)kmalloc(fsize, GFP_KERNEL);
+ if(fp == NULL)
+ return (-ENOMEM);
+
+ if (copy_from_user(fp, fprog->filter, fsize)) {
+ kfree_s(fp, fsize);
+ return -EFAULT;
+ }

- memset(fp,0,sizeof(*fp));
- memcpy(fp, fprog->filter, fsize); /* Copy instructions */
+ if((err = sk_chk_filter(fprog->filter, fprog->len))==0) {
+ /*
+ * Can't use lock_sock because some socket types
+ * don't use it.
+ */
+ start_bh_atomic();

+ old_filter = sk->filter_data;
sk->filter = fprog->len; /* Number of filter blocks */
sk->filter_data = fp; /* Filter instructions */
+
+ end_bh_atomic();
+
+ if (old_filter)
+ kfree(old_filter);
}

return (err);

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/