svn commit: r235746 - head/sys/net

[Alexander V. Chernikov]

On 24.05.2012 04:18, Jung-uk Kim wrote:
...
>> @@ -1664,7 +1668,7 @@ bpf_setf(struct bpf_d *d, struct bpf_pro
>> struct bpf_insn *fcode, *old; u_int wfilter, flen, size; #ifdef
>> BPF_JITTER -	bpf_jit_filter *ofunc; +	bpf_jit_filter *ofunc,
>> *jfunc; #endif int need_upgrade; #ifdef COMPAT_FREEBSD32 @@ -1695,6
>> +1699,13 @@ bpf_setf(struct bpf_d *d, struct bpf_pro else fcode =
>> NULL; /* Make compiler happy */
>>
>> +#ifdef BPF_JITTER +	if (fp->bf_insns != NULL) +		jfunc =
>> bpf_jitter(fcode, flen); +	else +		jfunc = NULL; /* Make compiler
>> happy */ +#endif + BPF_LOCK();
>>
>> if (cmd == BIOCSETWF) {
> ...
>
> This is completely wrong.  First, fcode has not been initialized with
Ups.

Does the attached patch look ok to you?
-------------- next part --------------
Index: sys/net/bpf.c
===================================================================
--- sys/net/bpf.c	(revision 235750)
+++ sys/net/bpf.c	(working copy)
@@ -1737,9 +1737,16 @@ bpf_setf(struct bpf_d *d, struct bpf_program *fp,
 
 	need_upgrade = 0;
 	size = flen * sizeof(*fp->bf_insns);
-	if (size > 0)
+	if (size > 0) {
+		/* We're setting up new filter. Copy and check actual data */
 		fcode = (struct bpf_insn *)malloc(size, M_BPF, M_WAITOK);
-	else
+		if (copyin((caddr_t)fp->bf_insns, (caddr_t)fcode, size) != 0 ||
+		    (bpf_validate(fcode, (int)flen) == 0)) {
+			free(fcode, M_BPF);
+			return (EINVAL);
+		}
+		/* Filter is copied inside fcode and is perfectly valid */
+	} else
 		fcode = NULL; /* Make compiler happy */
 
 #ifdef BPF_JITTER
@@ -1795,55 +1802,50 @@ bpf_setf(struct bpf_d *d, struct bpf_program *fp,
 		return (0);
 	}
 
-	if (copyin((caddr_t)fp->bf_insns, (caddr_t)fcode, size) == 0 &&
-	    bpf_validate(fcode, (int)flen)) {
-		/* 
-		 * Protect filter change by interface lock
-		 * Additionally, we are protected by global lock here.
-		 */
-		if (d->bd_bif != NULL)
-			BPFIF_WLOCK(d->bd_bif);
-		BPFD_LOCK(d);
-		if (wfilter)
-			d->bd_wfilter = fcode;
-		else {
-			d->bd_rfilter = fcode;
+	/* 
+	 * Set up new filter.
+	 * Protect filter change by interface lock
+	 * Additionally, we are protected by global lock here.
+	 */
+	if (d->bd_bif != NULL)
+		BPFIF_WLOCK(d->bd_bif);
+	BPFD_LOCK(d);
+	if (wfilter)
+		d->bd_wfilter = fcode;
+	else {
+		d->bd_rfilter = fcode;
 #ifdef BPF_JITTER
-			d->bd_bfilter = jfunc;
+		d->bd_bfilter = jfunc;
 #endif
-			if (cmd == BIOCSETF)
-				reset_d(d);
+		if (cmd == BIOCSETF)
+			reset_d(d);
 
-			/*
-			 * Do not require upgrade by first BIOCSETF
-			 * (used to set snaplen) by pcap_open_live()
-			 */
-			if ((d->bd_writer != 0) && (--d->bd_writer == 0))
-				need_upgrade = 1;
-			CTR4(KTR_NET, "%s: filter function set by pid %d, "
-			    "bd_writer counter %d, need_upgrade %d",
-			    __func__, d->bd_pid, d->bd_writer, need_upgrade);
-		}
-		BPFD_UNLOCK(d);
-		if (d->bd_bif != NULL)
-			BPFIF_WUNLOCK(d->bd_bif);
-		if (old != NULL)
-			free((caddr_t)old, M_BPF);
+		/*
+		 * Do not require upgrade by first BIOCSETF
+		 * (used to set snaplen) by pcap_open_live()
+		 */
+		if ((d->bd_writer != 0) && (--d->bd_writer == 0))
+			need_upgrade = 1;
+		CTR4(KTR_NET, "%s: filter function set by pid %d, "
+		    "bd_writer counter %d, need_upgrade %d",
+		    __func__, d->bd_pid, d->bd_writer, need_upgrade);
+	}
+	BPFD_UNLOCK(d);
+	if (d->bd_bif != NULL)
+		BPFIF_WUNLOCK(d->bd_bif);
+	if (old != NULL)
+		free((caddr_t)old, M_BPF);
 #ifdef BPF_JITTER
-		if (ofunc != NULL)
-			bpf_destroy_jit_filter(ofunc);
+	if (ofunc != NULL)
+		bpf_destroy_jit_filter(ofunc);
 #endif
 
-		/* Move d to active readers list */
-		if (need_upgrade != 0)
-			bpf_upgraded(d);
+	/* Move d to active readers list */
+	if (need_upgrade != 0)
+		bpf_upgraded(d);
 
-		BPF_UNLOCK();
-		return (0);
-	}
-	free((caddr_t)fcode, M_BPF);
 	BPF_UNLOCK();
-	return (EINVAL);
+	return (0);
 }
 
 /*
Index: sys/net/bpf_jitter.c
===================================================================
--- sys/net/bpf_jitter.c	(revision 235750)
+++ sys/net/bpf_jitter.c	(working copy)
@@ -71,7 +71,7 @@ bpf_jitter(struct bpf_insn *fp, int nins)
 	/* Allocate the filter structure. */
 #ifdef _KERNEL
 	filter = (struct bpf_jit_filter *)malloc(sizeof(*filter),
-	    M_BPFJIT, M_NOWAIT);
+	    M_BPFJIT, M_WAITOK);
 #else
 	filter = (struct bpf_jit_filter *)malloc(sizeof(*filter));
 #endif