Re: kern/48308: User can crash machine using a USB webcam

[Mihai Chelaru <mihai.chelaru%ngnetworks.ro@localhost>]

The following reply was made to PR kern/48308; it has been noted by GNATS.

From: Mihai Chelaru <mihai.chelaru%ngnetworks.ro@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: dtyson%anduin.org.uk@localhost
Subject: Re: kern/48308: User can crash machine using a USB webcam
Date: Mon, 04 Nov 2013 21:42:39 +0200

 This is a multi-part message in MIME format.
 --------------090805000104090603000906
 Content-Type: text/plain; charset=ISO-8859-1; format=flowed
 Content-Transfer-Encoding: 7bit
 
 Hi,
 
 Probably it works in 6.1 because release kernels are not compiled with 
 options DIAGNOSTIC, so they don't trigger that assert. I use the 
 attached patch for some time without any problems. It should fix your 
 issue too.
 
 -- 
 Mihai
 
 --------------090805000104090603000906
 Content-Type: text/plain; charset=us-ascii;
  name="usb_mem.c.diff"
 Content-Transfer-Encoding: 7bit
 Content-Disposition: attachment;
  filename="usb_mem.c.diff"
 
 Index: sys/dev/usb/usb_mem.c
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/usb/usb_mem.c,v
 retrieving revision 1.63
 diff -u -p -r1.63 usb_mem.c
 --- sys/dev/usb/usb_mem.c      15 Sep 2013 15:47:27 -0000      1.63
 +++ sys/dev/usb/usb_mem.c      4 Nov 2013 19:23:44 -0000
 @@ -44,6 +44,7 @@ __KERNEL_RCSID(0, "$NetBSD: usb_mem.c,v 
  #include <sys/systm.h>
  #include <sys/kernel.h>
  #include <sys/kmem.h>
 +#include <sys/pool.h>
  #include <sys/queue.h>
  #include <sys/device.h>               /* for usbdivar.h */
  #include <sys/bus.h>
 @@ -81,6 +82,8 @@ struct usb_frag_dma {
        LIST_ENTRY(usb_frag_dma) next;
  };
  
 +pool_cache_t  dma_block_pool, dma_sseg_pool;
 +
  Static usbd_status    usb_block_allocmem(bus_dma_tag_t, size_t, size_t,
                                           usb_dma_block_t **, bool);
  Static void           usb_block_freemem(usb_dma_block_t *);
 @@ -108,6 +111,21 @@ usb_mem_init(void)
  {
  
        mutex_init(&usb_blk_lock, MUTEX_DEFAULT, IPL_NONE);
 +
 +      dma_block_pool = pool_cache_init(sizeof(usb_dma_block_t), 0, 0, 0,
 +          "udmablock", NULL, IPL_VM, NULL, NULL, NULL);
 +      if (dma_block_pool == NULL)
 +              panic("dma_block_pool");
 +      pool_cache_sethiwat(dma_block_pool, 50);
 +      pool_cache_setlowat(dma_block_pool, 20);
 +
 +      dma_sseg_pool = pool_cache_init(sizeof(bus_dma_segment_t), 0, 0, 0,
 +          "udmasseg", NULL, IPL_VM, NULL, NULL, NULL);
 +      if (dma_sseg_pool == NULL)
 +              panic("dma_sseg_pool");
 +      pool_cache_sethiwat(dma_sseg_pool, 50);
 +      pool_cache_setlowat(dma_sseg_pool, 20);
 +
        return 0;
  }
  
 @@ -151,17 +169,11 @@ usb_block_allocmem(bus_dma_tag_t tag, si
                }
        }
  
 -#ifdef DIAGNOSTIC
 -      if (cpu_intr_p()) {
 -              printf("usb_block_allocmem: in interrupt context, failed\n");
 -              return (USBD_NOMEM);
 -      }
 -#endif
 -
        DPRINTFN(6, ("usb_block_allocmem: no free\n"));
 -      b = kmem_zalloc(sizeof *b, KM_SLEEP);
 +      b = pool_cache_get(dma_block_pool, PR_WAITOK);
        if (b == NULL)
                return (USBD_NOMEM);
 +      memset(b, 0, sizeof *b);
  
        b->tag = tag;
        b->size = size;
 @@ -173,11 +185,15 @@ usb_block_allocmem(bus_dma_tag_t tag, si
        else
                b->nsegs = (size + (PAGE_SIZE-1)) / PAGE_SIZE;
  
 -      b->segs = kmem_alloc(b->nsegs * sizeof(*b->segs), KM_SLEEP);
 +      if (b->nsegs == 1)
 +              b->segs = pool_cache_get(dma_sseg_pool, PR_WAITOK);
 +      else
 +              b->segs = kmem_intr_alloc(b->nsegs *sizeof(*b->segs), KM_SLEEP);
        if (b->segs == NULL) {
 -              kmem_free(b, sizeof *b);
 +              pool_cache_put(dma_block_pool, b);
                return USBD_NOMEM;
        }
 +      memset(b->segs, 0, b->nsegs * sizeof(*b->segs));
        b->nsegs_alloc = b->nsegs;
  
        error = bus_dmamem_alloc(tag, b->size, align, 0,
 @@ -215,8 +231,11 @@ usb_block_allocmem(bus_dma_tag_t tag, si
   free1:
        bus_dmamem_free(tag, b->segs, b->nsegs);
   free0:
 -      kmem_free(b->segs, b->nsegs_alloc * sizeof(*b->segs));
 -      kmem_free(b, sizeof *b);
 +      if (__predict_true(b->nsegs_alloc == 1))
 +              pool_cache_put(dma_sseg_pool, b->segs);
 +      else
 +              kmem_intr_free(b->segs, b->nsegs_alloc * sizeof(*b->segs));
 +      pool_cache_put(dma_block_pool, b);
        return (USBD_NOMEM);
  }
  
 @@ -234,8 +253,11 @@ usb_block_real_freemem(usb_dma_block_t *
        bus_dmamap_destroy(b->tag, b->map);
        bus_dmamem_unmap(b->tag, b->kaddr, b->size);
        bus_dmamem_free(b->tag, b->segs, b->nsegs);
 -      kmem_free(b->segs, b->nsegs_alloc * sizeof(*b->segs));
 -      kmem_free(b, sizeof *b);
 +      if (__predict_true(b->nsegs_alloc == 1))
 +              pool_cache_put(dma_sseg_pool, b->segs);
 +      else
 +              kmem_intr_free(b->segs, b->nsegs_alloc * sizeof(*b->segs));
 +      pool_cache_put(dma_block_pool, b);
  }
  #endif
  
 @@ -322,7 +344,7 @@ usb_allocmem_flags(usbd_bus_handle bus, 
        if (f == NULL) {
                DPRINTFN(1, ("usb_allocmem: adding fragments\n"));
                err = usb_block_allocmem(tag, USB_MEM_BLOCK, USB_MEM_SMALL, &b,
 -                                       false);
 +                                       0);
                if (err) {
                        mutex_exit(&usb_blk_lock);
                        return (err);
 @@ -438,21 +460,22 @@ usb_reserve_allocm(struct usb_dma_reserv
        if (rs->vaddr == 0 || size > USB_MEM_RESERVE)
                return USBD_NOMEM;
  
 -      dma->block = kmem_zalloc(sizeof *dma->block, KM_SLEEP);
 +      dma->block = pool_cache_get(dma_block_pool, PR_WAITOK);
        if (dma->block == NULL) {
                aprint_error_dev(rs->dv, "%s: failed allocating dma block",
                    __func__);
                goto out0;
        }
 +      memset(dma->block, 0, sizeof(*dma->block));
  
 -      dma->block->nsegs = 1;
 -      dma->block->segs = kmem_alloc(dma->block->nsegs *
 -          sizeof(*dma->block->segs), KM_SLEEP);
 +      dma->block->nsegs = dma->block->nsegs_alloc = 1;
 +      dma->block->segs = pool_cache_get(dma_sseg_pool, PR_WAITOK);
        if (dma->block->segs == NULL) {
                aprint_error_dev(rs->dv, "%s: failed allocating 1 dma segment",
                    __func__);
                goto out1;
        }
 +      memset(dma->block->segs, 0, sizeof(*dma->block->segs)); /* nsegs = 1 */
  
        error = extent_alloc(rs->extent, size, PAGE_SIZE, 0,
            EX_NOWAIT, &start);
 @@ -475,10 +498,9 @@ usb_reserve_allocm(struct usb_dma_reserv
  
        return USBD_NORMAL_COMPLETION;
  out2:
 -      kmem_free(dma->block->segs, dma->block->nsegs *
 -          sizeof(*dma->block->segs));
 +      pool_cache_put(dma_sseg_pool, dma->block->segs);
  out1:
 -      kmem_free(dma->block, sizeof *dma->block);
 +      pool_cache_put(dma_block_pool, dma->block);
  out0:
        return USBD_NOMEM;
  }
 @@ -489,9 +511,12 @@ usb_reserve_freem(struct usb_dma_reserve
  
        extent_free(rs->extent,
            (u_long)(rs->paddr + dma->offs), dma->block->size, 0);
 -      kmem_free(dma->block->segs, dma->block->nsegs *
 -          sizeof(*dma->block->segs));
 -      kmem_free(dma->block, sizeof *dma->block);
 +      if (__predict_true(dma->block->nsegs_alloc == 1))
 +              pool_cache_put(dma_sseg_pool, dma->block->segs);
 +      else
 +              kmem_intr_free(dma->block->segs, dma->block->nsegs *
 +                  sizeof(*dma->block->segs));
 +      pool_cache_put(dma_block_pool, dma->block);
  }
  
  int
 
 --------------090805000104090603000906--