[PATCH] sparse: sock_fprog sanitized

sock_fprog instances that had kernel pointer in ->filter (both of them -
in ppp_generic and isdn_ppp) replaced with explicit pairs len + kernel
pointer.  Copying of userland sock_fprog (with its __user ->filter)
cleaned up and sanitized.

Trivial annotation done in the rest of ->ioctl() in ppp_generic and
isdn_ppp.

drivers/isdn/i4l/isdn_ppp.h

@@ -12,8 +12,8 @@
 #include <linux/ppp_defs.h>     /* for PPP_PROTOCOL */
 #include <linux/isdn_ppp.h>	/* for isdn_ppp info */
 
-extern int isdn_ppp_read(int, struct file *, char *, int);
-extern int isdn_ppp_write(int, struct file *, const char *, int);
+extern int isdn_ppp_read(int, struct file *, char __user *, int);
+extern int isdn_ppp_write(int, struct file *, const char __user *, int);
 extern int isdn_ppp_open(int, struct file *);
 extern int isdn_ppp_init(void);
 extern void isdn_ppp_cleanup(void);

drivers/net/ppp_generic.c

@@ -129,8 +129,9 @@ struct ppp {
 #endif /* CONFIG_PPP_MULTILINK */
 	struct net_device_stats stats;	/* statistics */
 #ifdef CONFIG_PPP_FILTER
-	struct sock_fprog pass_filter;	/* filter for packets to pass */
-	struct sock_fprog active_filter;/* filter for pkts to reset idle */
+	struct sock_filter *pass_filter;	/* filter for packets to pass */
+	struct sock_filter *active_filter;/* filter for pkts to reset idle */
+	unsigned pass_len, active_len;
 #endif /* CONFIG_PPP_FILTER */
 };
 
@@ -493,6 +494,43 @@ static unsigned int ppp_poll(struct file *file, poll_table *wait)
 	return mask;
 }
 
+static int get_filter(void __user *arg, struct sock_filter **p)
+{
+	struct sock_fprog uprog;
+	struct sock_filter *code = NULL;
+	int len, err;
+
+	if (copy_from_user(&uprog, arg, sizeof(uprog)))
+		return -EFAULT;
+
+	if (uprog.len > BPF_MAXINSNS)
+		return -EINVAL;
+
+	if (!uprog.len) {
+		*p = NULL;
+		return 0;
+	}
+
+	len = uprog.len * sizeof(struct sock_filter);
+	code = kmalloc(len, GFP_KERNEL);
+	if (code == NULL)
+		return -ENOMEM;
+
+	if (copy_from_user(code, uprog.filter, len)) {
+		kfree(code);
+		return -EFAULT;
+	}
+
+	err = sk_chk_filter(code, uprog.len);
+	if (err) {
+		kfree(code);
+		return err;
+	}
+
+	*p = code;
+	return uprog.len;
+}
+
 static int ppp_ioctl(struct inode *inode, struct file *file,
 		     unsigned int cmd, unsigned long arg)
 {
@@ -503,6 +541,8 @@ static int ppp_ioctl(struct inode *inode, struct file *file,
 	struct npioctl npi;
 	int unit, cflags;
 	struct slcompress *vj;
+	void __user *argp = (void __user *)arg;
+	int __user *p = argp;
 
 	if (pf == 0)
 		return ppp_unattached_ioctl(pf, file, cmd, arg);
@@ -540,7 +580,7 @@ static int ppp_ioctl(struct inode *inode, struct file *file,
 
 		switch (cmd) {
 		case PPPIOCCONNECT:
-			if (get_user(unit, (int *) arg))
+			if (get_user(unit, p))
 				break;
 			err = ppp_connect_channel(pch, unit);
 			break;
@@ -569,14 +609,14 @@ static int ppp_ioctl(struct inode *inode, struct file *file,
 	ppp = PF_TO_PPP(pf);
 	switch (cmd) {
 	case PPPIOCSMRU:
-		if (get_user(val, (int *) arg))
+		if (get_user(val, p))
 			break;
 		ppp->mru = val;
 		err = 0;
 		break;
 
 	case PPPIOCSFLAGS:
-		if (get_user(val, (int *) arg))
+		if (get_user(val, p))
 			break;
 		ppp_lock(ppp);
 		cflags = ppp->flags & ~val;
@@ -589,7 +629,7 @@ static int ppp_ioctl(struct inode *inode, struct file *file,
 
 	case PPPIOCGFLAGS:
 		val = ppp->flags | ppp->xstate | ppp->rstate;
-		if (put_user(val, (int *) arg))
+		if (put_user(val, p))
 			break;
 		err = 0;
 		break;
@@ -599,20 +639,20 @@ static int ppp_ioctl(struct inode *inode, struct file *file,
 		break;
 
 	case PPPIOCGUNIT:
-		if (put_user(ppp->file.index, (int *) arg))
+		if (put_user(ppp->file.index, p))
 			break;
 		err = 0;
 		break;
 
 	case PPPIOCSDEBUG:
-		if (get_user(val, (int *) arg))
+		if (get_user(val, p))
 			break;
 		ppp->debug = val;
 		err = 0;
 		break;
 
 	case PPPIOCGDEBUG:
-		if (put_user(ppp->debug, (int *) arg))
+		if (put_user(ppp->debug, p))
 			break;
 		err = 0;
 		break;
@@ -620,13 +660,13 @@ static int ppp_ioctl(struct inode *inode, struct file *file,
 	case PPPIOCGIDLE:
 		idle.xmit_idle = (jiffies - ppp->last_xmit) / HZ;
 		idle.recv_idle = (jiffies - ppp->last_recv) / HZ;
-		if (copy_to_user((void __user *) arg, &idle, sizeof(idle)))
+		if (copy_to_user(argp, &idle, sizeof(idle)))
 			break;
 		err = 0;
 		break;
 
 	case PPPIOCSMAXCID:
-		if (get_user(val, (int *) arg))
+		if (get_user(val, p))
 			break;
 		val2 = 15;
 		if ((val >> 16) != 0) {
@@ -649,7 +689,7 @@ static int ppp_ioctl(struct inode *inode, struct file *file,
 
 	case PPPIOCGNPMODE:
 	case PPPIOCSNPMODE:
-		if (copy_from_user(&npi, (void __user *) arg, sizeof(npi)))
+		if (copy_from_user(&npi, argp, sizeof(npi)))
 			break;
 		err = proto_to_npindex(npi.protocol);
 		if (err < 0)
@@ -658,7 +698,7 @@ static int ppp_ioctl(struct inode *inode, struct file *file,
 		if (cmd == PPPIOCGNPMODE) {
 			err = -EFAULT;
 			npi.mode = ppp->npmode[i];
-			if (copy_to_user((void __user *) arg, &npi, sizeof(npi)))
+			if (copy_to_user(argp, &npi, sizeof(npi)))
 				break;
 		} else {
 			ppp->npmode[i] = npi.mode;
@@ -670,49 +710,38 @@ static int ppp_ioctl(struct inode *inode, struct file *file,
 
 #ifdef CONFIG_PPP_FILTER
 	case PPPIOCSPASS:
+	{
+		struct sock_filter *code;
+		err = get_filter(argp, &code);
+		if (err >= 0) {
+			ppp_lock(ppp);
+			kfree(ppp->pass_filter);
+			ppp->pass_filter = code;
+			ppp->pass_len = err;
+			ppp_unlock(ppp);
+			err = 0;
+		}
+		break;
+	}
 	case PPPIOCSACTIVE:
 	{
-		struct sock_fprog uprog, *filtp;
-		struct sock_filter *code = NULL;
-		int len;
-
-		if (copy_from_user(&uprog, (void __user *) arg, sizeof(uprog)))
-			break;
-		err = -EINVAL;
-		if (uprog.len > BPF_MAXINSNS)
-			break;
-		err = -ENOMEM;
-		if (uprog.len > 0) {
-			len = uprog.len * sizeof(struct sock_filter);
-			code = kmalloc(len, GFP_KERNEL);
-			if (code == NULL)
-				break;
-			err = -EFAULT;
-			if (copy_from_user(code, (void __user *) uprog.filter, len)) {
-				kfree(code);
-				break;
-			}
-			err = sk_chk_filter(code, uprog.len);
-			if (err) {
-				kfree(code);
-				break;
-			}
+		struct sock_filter *code;
+		err = get_filter(argp, &code);
+		if (err >= 0) {
+			ppp_lock(ppp);
+			kfree(ppp->active_filter);
+			ppp->active_filter = code;
+			ppp->active_len = err;
+			ppp_unlock(ppp);
+			err = 0;
 		}
-		filtp = (cmd == PPPIOCSPASS)? &ppp->pass_filter: &ppp->active_filter;
-		ppp_lock(ppp);
-		if (filtp->filter)
-			kfree(filtp->filter);
-		filtp->filter = code;
-		filtp->len = uprog.len;
-		ppp_unlock(ppp);
-		err = 0;
 		break;
 	}
 #endif /* CONFIG_PPP_FILTER */
 
 #ifdef CONFIG_PPP_MULTILINK
 	case PPPIOCSMRRU:
-		if (get_user(val, (int *) arg))
+		if (get_user(val, p))
 			break;
 		ppp_recv_lock(ppp);
 		ppp->mrru = val;
@@ -734,11 +763,12 @@ static int ppp_unattached_ioctl(struct ppp_file *pf, struct file *file,
 	int unit, err = -EFAULT;
 	struct ppp *ppp;
 	struct channel *chan;
+	int __user *p = (int __user *)arg;
 
 	switch (cmd) {
 	case PPPIOCNEWUNIT:
 		/* Create a new ppp unit */
-		if (get_user(unit, (int *) arg))
+		if (get_user(unit, p))
 			break;
 		ppp = ppp_create_interface(unit, &err);
 		if (ppp == 0)
@@ -746,14 +776,14 @@ static int ppp_unattached_ioctl(struct ppp_file *pf, struct file *file,
 		file->private_data = &ppp->file;
 		ppp->owner = file;
 		err = -EFAULT;
-		if (put_user(ppp->file.index, (int *) arg))
+		if (put_user(ppp->file.index, p))
 			break;
 		err = 0;
 		break;
 
 	case PPPIOCATTACH:
 		/* Attach to an existing ppp unit */
-		if (get_user(unit, (int *) arg))
+		if (get_user(unit, p))
 			break;
 		down(&all_ppp_sem);
 		err = -ENXIO;
@@ -767,7 +797,7 @@ static int ppp_unattached_ioctl(struct ppp_file *pf, struct file *file,
 		break;
 
 	case PPPIOCATTCHAN:
-		if (get_user(unit, (int *) arg))
+		if (get_user(unit, p))
 			break;
 		spin_lock_bh(&all_channels_lock);
 		err = -ENXIO;
@@ -999,18 +1029,18 @@ ppp_send_frame(struct ppp *ppp, struct sk_buff *skb)
 
 			*p = htons(4); /* indicate outbound in DLT_LINUX_SLL */;
 		}
-		if (ppp->pass_filter.filter
-		    && sk_run_filter(skb, ppp->pass_filter.filter,
-				     ppp->pass_filter.len) == 0) {
+		if (ppp->pass_filter
+		    && sk_run_filter(skb, ppp->pass_filter,
+				     ppp->pass_len) == 0) {
 			if (ppp->debug & 1)
 				printk(KERN_DEBUG "PPP: outbound frame not passed\n");
 			kfree_skb(skb);
 			return;
 		}
 		/* if this packet passes the active filter, record the time */
-		if (!(ppp->active_filter.filter
-		      && sk_run_filter(skb, ppp->active_filter.filter,
-				       ppp->active_filter.len) == 0))
+		if (!(ppp->active_filter
+		      && sk_run_filter(skb, ppp->active_filter,
+				       ppp->active_len) == 0))
 			ppp->last_xmit = jiffies;
 		skb_pull(skb, 2);
 #else
@@ -1546,17 +1576,17 @@ ppp_receive_nonmp_frame(struct ppp *ppp, struct sk_buff *skb)
 
 			*p = 0; /* indicate inbound in DLT_LINUX_SLL */
 		}
-		if (ppp->pass_filter.filter
-		    && sk_run_filter(skb, ppp->pass_filter.filter,
-				     ppp->pass_filter.len) == 0) {
+		if (ppp->pass_filter
+		    && sk_run_filter(skb, ppp->pass_filter,
+				     ppp->pass_len) == 0) {
 			if (ppp->debug & 1)
 				printk(KERN_DEBUG "PPP: inbound frame not passed\n");
 			kfree_skb(skb);
 			return;
 		}
-		if (!(ppp->active_filter.filter
-		      && sk_run_filter(skb, ppp->active_filter.filter,
-				       ppp->active_filter.len) == 0))
+		if (!(ppp->active_filter
+		      && sk_run_filter(skb, ppp->active_filter,
+				       ppp->active_len) == 0))
 			ppp->last_recv = jiffies;
 		skb_pull(skb, 2);
 #else
@@ -2423,13 +2453,13 @@ static void ppp_destroy_interface(struct ppp *ppp)
 	skb_queue_purge(&ppp->mrq);
 #endif /* CONFIG_PPP_MULTILINK */
 #ifdef CONFIG_PPP_FILTER
-	if (ppp->pass_filter.filter) {
-		kfree(ppp->pass_filter.filter);
-		ppp->pass_filter.filter = NULL;
+	if (ppp->pass_filter) {
+		kfree(ppp->pass_filter);
+		ppp->pass_filter = NULL;
 	}
-	if (ppp->active_filter.filter) {
-		kfree(ppp->active_filter.filter);
-		ppp->active_filter.filter = 0;
+	if (ppp->active_filter) {
+		kfree(ppp->active_filter);
+		ppp->active_filter = 0;
 	}
 #endif /* CONFIG_PPP_FILTER */
 

include/linux/isdn_ppp.h

@@ -233,8 +233,9 @@ struct ippp_struct {
   struct slcompress *slcomp;
 #endif
 #ifdef CONFIG_IPPP_FILTER
-  struct sock_fprog pass_filter;	/* filter for packets to pass */
-  struct sock_fprog active_filter;	/* filter for pkts to reset idle */
+  struct sock_filter *pass_filter;	/* filter for packets to pass */
+  struct sock_filter *active_filter;	/* filter for pkts to reset idle */
+  unsigned pass_len, active_len;
 #endif
   unsigned long debug;
   struct isdn_ppp_compressor *compressor,*decompressor;