Hi!

I have finally discovered, what makes n*gratv fail for
me with kernel oopses:

The current bttv implementation of close and mmap has
some serious security and stability problems:
(abstract: rvfree at bttv_close() time is not a good idea,
bt848_set_risc_jmps(btv) in bttv_close() does not prevent the
grabber from grabbing into the rvfree'ed memory)

security: one can close /dev/videox and have still mappings
of the buffers. But as close deallocates them, the user can
now read (and write) to pages used probably by other processes
or the kernel. A program could open, mmap, close many times
and get just every ram mapped into its vm space. This should
be fixed by freeing the buffer at munmap time. For this
reason, videodev.c:video_mmap has to pass vm_area_struct
to the driver so it can use vmops->unmap to track the unmap
case. (I think we must ensure, the module is present during
a mmapped buffer, too) - as this would change the present
interface there is no patch yet...

stability: one can close /dev/videox and have still outstanding
grab requests, so after freeing the pages, they could be
reused and corrupted by the grabber. This should be fixed
(as it could in the userspace prgs by issuing two syncs at
quit time) by stopping the grabber some way (the current way
does not work) or waiting 40ms (ugly) - no patch, too.

The above stated problems could be easily reproduced by the
attached program (umount your big hd's before doing the
memory-corruption one :)). Just issue while true; do ./test; done
and do some memory stress to the machine. Sooner or later (even
after finishing ./test) you get a hard lockup. The security
part shows, that after clearing the buffer and closing the
device, the memory gets modified (by an other process or the
kernel).

Richard.

PS: all tests were done with the bttv driver shipped with
    xawtv-2.19, but as the offending code did not change
    in the experimental driver, it should have still the
    same problems.


#include <linux/types.h>
#include <linux/videodev.h>
#include <sys/mman.h>
#include <sys/ioctl.h>
#include <sys/time.h>
#include <unistd.h>
#include <stdio.h>
#include <fcntl.h>

#define BTTV_MAX_FBUF 0x151000

int access_mem(char *mem, int size)
{
  int i, retval=0;
  volatile int j;

  for (i=0; i<size; i++)
    if ((j = ((volatile char *)mem)[i]) != 0)
      return retval = 1;
  return retval;
}

int main(int argc, char **argv)
{
  int fd, cnt, g0, g1;
  char *map;
  struct video_mmap vmap;
  struct timeval start, stop;

  if ((fd = open("/dev/video0", O_RDWR)) == -1) {
    perror("open /dev/video0");
    goto _out;
  }

  map = mmap(0, BTTV_MAX_FBUF*2, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
  if ((int)map == -1) {
    perror("mmap");
    goto _close_vid;
  }
  memset(map, 0, BTTV_MAX_FBUF*2);

  vmap.height = 576;
  vmap.width = 768;
  vmap.format = 0x22;


#if 1
  /*
   * show security flaw (read mem after close)
   * ideally, we could loop over open(), mmap() and close()
   * and get just every mem mapped to our process after
   * some time??
   */
  if (close(fd) == -1)
    perror("close");
  cnt = 0;
  while (1) {
    if (access_mem(map, BTTV_MAX_FBUF*2)) {
      fprintf(stderr, "ARGH! mem modified after %i runs!\n", cnt);
      break;
    }
    cnt++;
  }
#endif


#if 0
  /*
   * show memory corruption!! (be careful!)
   */
  gettimeofday(&start, NULL);
  vmap.frame = 0;
  if (ioctl(fd, VIDIOCMCAPTURE, &vmap) == -1)
    perror("VIDIOCMCAPTURE");
  vmap.frame = 1;
  if (ioctl(fd, VIDIOCMCAPTURE, &vmap) == -1)
    perror("VIDIOCMCAPTURE");
  if (close(fd) == -1)
    perror("close");
  gettimeofday(&stop, NULL);

  /* here, grabbing should just have started or never start */
  g0 = access_mem(map, BTTV_MAX_FBUF);
  g1 = access_mem(map+BTTV_MAX_FBUF, BTTV_MAX_FBUF);

  fprintf(stderr, "2 times VIDIOCMCAPTURE + close lasts %i us\n",
	  (stop.tv_sec-start.tv_sec)*1000000
	  + (stop.tv_usec-start.tv_usec));
  usleep(2*40000);

  /* here grabbing should be finished */
  if (g1 != access_mem(map+BTTV_MAX_FBUF, BTTV_MAX_FBUF))
    fprintf(stderr, "Grabbing to frame 1 started after close!??\n");
  if (g0 != access_mem(map, BTTV_MAX_FBUF))
    fprintf(stderr, "Grabbing to frame 0 started after close!??\n");
#endif

_munmap_fbuf:
  if (munmap(map, BTTV_MAX_FBUF*2) == -1)
    perror("munmap");
_close_vid:
  if (close(fd) == -1)
    perror("close");
_out:
  return 0;
}


------------
To unsubscribe from this list send mail to majordomo@phunk.org with the
line "unsubscribe video4linux" without the quotes in the body of the
message.