On Wed, Jun 22, 2011 at 09:32:06PM +0200, Ariane van der Steldt wrote:
> On Tue, Jun 21, 2011 at 09:00:49PM +0200, Ariane van der Steldt wrote:
> > Bus_dmamem_map has a bug in its error path, where it frees the wrong
> > memory in the wrong way.
>
> After some discussion on icb, the comments and the pmap_remove can go
> too. The pmap_remove is executed by uvm_km_free() at uvm_unmap_remove()
> and uvm_km_free won't use the pmap but the object to lookup pages (and
> the object has none at these addresses).
>
> Ok?
OK.
> --
> Ariane
>
>
> Index: arch/alpha/dev/bus_dma.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/alpha/dev/bus_dma.c,v
> retrieving revision 1.30
> diff -u -d -p -r1.30 bus_dma.c
> --- arch/alpha/dev/bus_dma.c 26 Dec 2010 15:40:58 -0000 1.30
> +++ arch/alpha/dev/bus_dma.c 22 Jun 2011 18:59:28 -0000
> @@ -614,12 +614,8 @@ _bus_dmamem_map(t, segs, nsegs, size, kv
> VM_PROT_READ | VM_PROT_WRITE, VM_PROT_READ |
> VM_PROT_WRITE | PMAP_WIRED | PMAP_CANFAIL);
> if (error) {
> - /*
> - * Clean up after ourselves.
> - * XXX uvm_wait on WAITOK
> - */
> pmap_update(pmap_kernel());
> - uvm_km_free(kernel_map, va, ssize);
> + uvm_km_free(kernel_map, sva, ssize);
> return (error);
> }
> }
> Index: arch/amd64/amd64/bus_dma.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/amd64/amd64/bus_dma.c,v
> retrieving revision 1.36
> diff -u -d -p -r1.36 bus_dma.c
> --- arch/amd64/amd64/bus_dma.c 2 Apr 2011 16:37:39 -0000 1.36
> +++ arch/amd64/amd64/bus_dma.c 22 Jun 2011 18:59:28 -0000
> @@ -492,12 +492,8 @@ _bus_dmamem_map(bus_dma_tag_t t, bus_dma
> VM_PROT_READ | VM_PROT_WRITE, VM_PROT_READ |
> VM_PROT_WRITE | PMAP_WIRED | PMAP_CANFAIL);
> if (error) {
> - /*
> - * Clean up after ourselves.
> - * XXX uvm_wait on WAITOK
> - */
> pmap_update(pmap_kernel());
> - uvm_km_free(kernel_map, va, ssize);
> + uvm_km_free(kernel_map, sva, ssize);
> return (error);
> }
> }
> Index: arch/arm/arm/bus_dma.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/arm/arm/bus_dma.c,v
> retrieving revision 1.20
> diff -u -d -p -r1.20 bus_dma.c
> --- arch/arm/arm/bus_dma.c 4 Jan 2011 21:12:55 -0000 1.20
> +++ arch/arm/arm/bus_dma.c 22 Jun 2011 18:59:30 -0000
> @@ -718,12 +718,8 @@ _bus_dmamem_map(bus_dma_tag_t t, bus_dma
> VM_PROT_READ | VM_PROT_WRITE, VM_PROT_READ |
> VM_PROT_WRITE | PMAP_WIRED | PMAP_CANFAIL);
> if (error) {
> - /*
> - * Clean up after ourselves.
> - * XXX uvm_wait on WAITOK
> - */
> pmap_update(pmap_kernel());
> - uvm_km_free(kernel_map, va, ssize);
> + uvm_km_free(kernel_map, sva, ssize);
> return (error);
> }
> /*
> Index: arch/aviion/aviion/bus_dma.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/aviion/aviion/bus_dma.c,v
> retrieving revision 1.3
> diff -u -d -p -r1.3 bus_dma.c
> --- arch/aviion/aviion/bus_dma.c 26 Dec 2010 15:40:59 -0000 1.3
> +++ arch/aviion/aviion/bus_dma.c 22 Jun 2011 18:59:30 -0000
> @@ -544,12 +544,8 @@ bus_dmamem_map(t, segs, nsegs, size, kva
> VM_PROT_READ | VM_PROT_WRITE, VM_PROT_READ |
> VM_PROT_WRITE | PMAP_WIRED | PMAP_CANFAIL);
> if (error) {
> - /*
> - * Clean up after ourselves.
> - * XXX uvm_wait on WAITOK
> - */
> pmap_update(pmap_kernel());
> - uvm_km_free(kernel_map, va, ssize);
> + uvm_km_free(kernel_map, sva, ssize);
> return (error);
> }
> }
> Index: arch/i386/i386/bus_dma.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/i386/i386/bus_dma.c,v
> retrieving revision 1.24
> diff -u -d -p -r1.24 bus_dma.c
> --- arch/i386/i386/bus_dma.c 26 Dec 2010 15:40:59 -0000 1.24
> +++ arch/i386/i386/bus_dma.c 22 Jun 2011 18:59:30 -0000
> @@ -456,14 +456,8 @@ _bus_dmamem_map(bus_dma_tag_t t, bus_dma
> VM_PROT_READ | VM_PROT_WRITE, VM_PROT_READ |
> VM_PROT_WRITE | PMAP_WIRED | PMAP_CANFAIL);
> if (ret) {
> - /*
> - * Clean up after ourselves.
> - * XXX uvm_wait on WAITOK
> - */
> - pmap_remove(pmap_kernel(), sva, va - PAGE_SIZE);
> pmap_update(pmap_kernel());
> -
> - uvm_km_free(kernel_map, va, ssize);
> + uvm_km_free(kernel_map, sva, ssize);
> return (ret);
> }
>
> Index: arch/loongson/loongson/bus_dma.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/loongson/loongson/bus_dma.c,v
> retrieving revision 1.6
> diff -u -d -p -r1.6 bus_dma.c
> --- arch/loongson/loongson/bus_dma.c 26 Dec 2010 15:40:59 -0000 1.6
> +++ arch/loongson/loongson/bus_dma.c 22 Jun 2011 18:59:32 -0000
> @@ -466,12 +466,8 @@ _dmamem_map(bus_dma_tag_t t, bus_dma_seg
> VM_PROT_READ | VM_PROT_WRITE, VM_PROT_READ |
> VM_PROT_WRITE | PMAP_WIRED | PMAP_CANFAIL);
> if (error) {
> - /*
> - * Clean up after ourselves.
> - * XXX uvm_wait on WAITOK
> - */
> pmap_update(pmap_kernel());
> - uvm_km_free(kernel_map, va, ssize);
> + uvm_km_free(kernel_map, sva, ssize);
> return (error);
> }
>
> Index: arch/macppc/macppc/dma.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/macppc/macppc/dma.c,v
> retrieving revision 1.34
> diff -u -d -p -r1.34 dma.c
> --- arch/macppc/macppc/dma.c 26 Dec 2010 15:40:59 -0000 1.34
> +++ arch/macppc/macppc/dma.c 22 Jun 2011 18:59:32 -0000
> @@ -483,12 +483,8 @@ _dmamem_map(bus_dma_tag_t t, bus_dma_seg
> VM_PROT_READ | VM_PROT_WRITE, VM_PROT_READ |
> VM_PROT_WRITE | PMAP_WIRED | PMAP_CANFAIL);
> if (error) {
> - /*
> - * Clean up after ourselves.
> - * XXX uvm_wait on WAITOK
> - */
> pmap_update(pmap_kernel());
> - uvm_km_free(kernel_map, va, ssize);
> + uvm_km_free(kernel_map, sva, ssize);
> return (error);
> }
> }
> Index: arch/mvme68k/mvme68k/bus_dma.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/mvme68k/mvme68k/bus_dma.c,v
> retrieving revision 1.8
> diff -u -d -p -r1.8 bus_dma.c
> --- arch/mvme68k/mvme68k/bus_dma.c 26 Dec 2010 15:40:59 -0000 1.8
> +++ arch/mvme68k/mvme68k/bus_dma.c 22 Jun 2011 18:59:32 -0000
> @@ -541,12 +541,8 @@ bus_dmamem_map(t, segs, nsegs, size, kva
> VM_PROT_READ | VM_PROT_WRITE, VM_PROT_READ |
> VM_PROT_WRITE | PMAP_WIRED | PMAP_CANFAIL);
> if (error) {
> - /*
> - * Clean up after ourselves.
> - * XXX uvm_wait on WAITOK
> - */
> pmap_update(pmap_kernel());
> - uvm_km_free(kernel_map, va, ssize);
> + uvm_km_free(kernel_map, sva, ssize);
> return (error);
> }
> }
> Index: arch/mvme88k/mvme88k/bus_dma.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/mvme88k/mvme88k/bus_dma.c,v
> retrieving revision 1.16
> diff -u -d -p -r1.16 bus_dma.c
> --- arch/mvme88k/mvme88k/bus_dma.c 26 Dec 2010 15:40:59 -0000 1.16
> +++ arch/mvme88k/mvme88k/bus_dma.c 22 Jun 2011 18:59:32 -0000
> @@ -544,12 +544,8 @@ bus_dmamem_map(t, segs, nsegs, size, kva
> VM_PROT_READ | VM_PROT_WRITE, VM_PROT_READ |
> VM_PROT_WRITE | PMAP_WIRED | PMAP_CANFAIL);
> if (error) {
> - /*
> - * Clean up after ourselves.
> - * XXX uvm_wait on WAITOK
> - */
> pmap_update(pmap_kernel());
> - uvm_km_free(kernel_map, va, ssize);
> + uvm_km_free(kernel_map, sva, ssize);
> return (error);
> }
> }
> Index: arch/mvmeppc/mvmeppc/bus_dma.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/mvmeppc/mvmeppc/bus_dma.c,v
> retrieving revision 1.28
> diff -u -d -p -r1.28 bus_dma.c
> --- arch/mvmeppc/mvmeppc/bus_dma.c 26 Dec 2010 15:40:59 -0000 1.28
> +++ arch/mvmeppc/mvmeppc/bus_dma.c 22 Jun 2011 18:59:32 -0000
> @@ -512,12 +512,8 @@ _bus_dmamem_map(t, segs, nsegs, size, kv
> VM_PROT_READ | VM_PROT_WRITE, VM_PROT_READ |
> VM_PROT_WRITE | PMAP_WIRED | PMAP_CANFAIL);
> if (error) {
> - /*
> - * Clean up after ourselves.
> - * XXX uvm_wait on WAITOK
> - */
> pmap_update(pmap_kernel());
> - uvm_km_free(kernel_map, va, ssize);
> + uvm_km_free(kernel_map, sva, ssize);
> return (error);
> }
> }
> Index: arch/octeon/octeon/bus_dma.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/octeon/octeon/bus_dma.c,v
> retrieving revision 1.2
> diff -u -d -p -r1.2 bus_dma.c
> --- arch/octeon/octeon/bus_dma.c 26 Dec 2010 15:41:00 -0000 1.2
> +++ arch/octeon/octeon/bus_dma.c 22 Jun 2011 18:59:33 -0000
> @@ -471,12 +471,8 @@ _dmamem_map(bus_dma_tag_t t, bus_dma_seg
> VM_PROT_READ | VM_PROT_WRITE, VM_PROT_READ |
> VM_PROT_WRITE | PMAP_WIRED | PMAP_CANFAIL);
> if (error) {
> - /*
> - * Clean up after ourselves.
> - * XXX uvm_wait on WAITOK
> - */
> pmap_update(pmap_kernel());
> - uvm_km_free(kernel_map, va, ssize);
> + uvm_km_free(kernel_map, sva, ssize);
> return (error);
> }
>
> Index: arch/sgi/sgi/bus_dma.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/sgi/sgi/bus_dma.c,v
> retrieving revision 1.21
> diff -u -d -p -r1.21 bus_dma.c
> --- arch/sgi/sgi/bus_dma.c 3 Apr 2011 22:33:55 -0000 1.21
> +++ arch/sgi/sgi/bus_dma.c 22 Jun 2011 18:59:33 -0000
> @@ -471,12 +471,8 @@ _dmamem_map(bus_dma_tag_t t, bus_dma_seg
> VM_PROT_READ | VM_PROT_WRITE, VM_PROT_READ |
> VM_PROT_WRITE | PMAP_WIRED | PMAP_CANFAIL);
> if (error) {
> - /*
> - * Clean up after ourselves.
> - * XXX uvm_wait on WAITOK
> - */
> pmap_update(pmap_kernel());
> - uvm_km_free(kernel_map, va, ssize);
> + uvm_km_free(kernel_map, sva, ssize);
> return (error);
> }
>
> Index: arch/socppc/socppc/dma.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/socppc/socppc/dma.c,v
> retrieving revision 1.8
> diff -u -d -p -r1.8 dma.c
> --- arch/socppc/socppc/dma.c 26 Dec 2010 15:41:00 -0000 1.8
> +++ arch/socppc/socppc/dma.c 22 Jun 2011 18:59:33 -0000
> @@ -483,12 +483,8 @@ _dmamem_map(bus_dma_tag_t t, bus_dma_seg
> VM_PROT_READ | VM_PROT_WRITE, VM_PROT_READ |
> VM_PROT_WRITE | PMAP_WIRED | PMAP_CANFAIL);
> if (error) {
> - /*
> - * Clean up after ourselves.
> - * XXX uvm_wait on WAITOK
> - */
> pmap_update(pmap_kernel());
> - uvm_km_free(kernel_map, va, ssize);
> + uvm_km_free(kernel_map, sva, ssize);
> return (error);
> }
> }
> Index: arch/sparc64/sparc64/machdep.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/sparc64/sparc64/machdep.c,v
> retrieving revision 1.132
> diff -u -d -p -r1.132 machdep.c
> --- arch/sparc64/sparc64/machdep.c 5 Jun 2011 19:41:08 -0000 1.132
> +++ arch/sparc64/sparc64/machdep.c 22 Jun 2011 18:59:35 -0000
> @@ -1494,12 +1494,8 @@ _bus_dmamem_map(t, t0, segs, nsegs, size
> VM_PROT_READ | VM_PROT_WRITE, VM_PROT_READ |
> VM_PROT_WRITE | PMAP_WIRED | PMAP_CANFAIL);
> if (error) {
> - /*
> - * Clean up after ourselves.
> - * XXX uvm_wait on WAITOK
> - */
> pmap_update(pmap_kernel());
> - uvm_km_free(kernel_map, va, ssize);
> + uvm_km_free(kernel_map, sva, ssize);
> return (error);
> }
> va += PAGE_SIZE;
> Index: arch/vax/vax/bus_dma.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/vax/vax/bus_dma.c,v
> retrieving revision 1.26
> diff -u -d -p -r1.26 bus_dma.c
> --- arch/vax/vax/bus_dma.c 26 Dec 2010 15:41:00 -0000 1.26
> +++ arch/vax/vax/bus_dma.c 22 Jun 2011 18:59:35 -0000
> @@ -469,12 +469,8 @@ _bus_dmamem_map(t, segs, nsegs, size, kv
> VM_PROT_READ | VM_PROT_WRITE, VM_PROT_READ |
> VM_PROT_WRITE | PMAP_WIRED | PMAP_CANFAIL);
> if (error) {
> - /*
> - * Clean up after ourselves.
> - * XXX uvm_wait on WAITOK
> - */
> pmap_update(pmap_kernel());
> - uvm_km_free(kernel_map, va, ssize);
> + uvm_km_free(kernel_map, sva, ssize);
> return (error);
> }
> }
>
--
I had to hit him -- he was starting to make sense.