Bug#542204: libv4l-0: libv4lconvert.so.0 creates shm files inaccessible by other users

[Gregor Jasny]

Hans de Goede wrote:
> Patches welcome. please let let me know if you do not plan on working on
> this then I'll do a patch myself.

The attached patch prefixes the shm segment name with the effective uid.

It falls back to malloc if shm segment creation fails. I'm unsure if it
would be better to set data->controls and data->shm_values both to zero.

You can find a patched source package at mentors and alioth:
http://mentors.debian.net/debian/pool/main/l/libv4l/libv4l_0.6.0-2.dsc
http://git.debian.org/?p=collab-maint/libv4l.git

Cheers,
Gregor

diff --git a/libv4lconvert/control/libv4lcontrol-priv.h b/libv4lconvert/control/libv4lcontrol-priv.h
index 3394132..9c19b34 100644
--- a/libv4lconvert/control/libv4lcontrol-priv.h
+++ b/libv4lconvert/control/libv4lcontrol-priv.h
@@ -25,6 +25,7 @@
 #define V4LCONTROL_SHM_SIZE 4096
 
 #define V4LCONTROL_SUPPORTS_NEXT_CTRL 0x01
+#define V4LCONTROL_MEMORY_IS_MALLOCED 0x02
 
 struct v4lcontrol_flags_info;
 
diff --git a/libv4lconvert/control/libv4lcontrol.c b/libv4lconvert/control/libv4lcontrol.c
index 7da1fb1..e9462ac 100644
--- a/libv4lconvert/control/libv4lcontrol.c
+++ b/libv4lconvert/control/libv4lcontrol.c
@@ -311,7 +311,7 @@ struct v4lcontrol_data *v4lcontrol_create(int fd, int always_needs_conversion)
     return data; /* No need to create a shared memory segment */
 
   SYS_IOCTL(fd, VIDIOC_QUERYCAP, &cap);
-  snprintf(shm_name, 256, "/%s:%s", cap.bus_info, cap.card);
+  snprintf(shm_name, 256, "/%lu-%s:%s", (unsigned long)geteuid(), cap.bus_info, cap.card);
 
   /* / is not allowed inside shm names */
   for (i = 1; shm_name[i]; i++)
@@ -322,23 +322,35 @@ struct v4lcontrol_data *v4lcontrol_create(int fd, int always_needs_conversion)
   if ((shm_fd = shm_open(shm_name, (O_CREAT | O_EXCL | O_RDWR),
 			 (S_IREAD | S_IWRITE))) >= 0)
     init = 1;
-  else if ((shm_fd = shm_open(shm_name, O_RDWR, (S_IREAD | S_IWRITE))) < 0)
-    goto error;
-
-  /* Set the shared memory size */
-  ftruncate(shm_fd, V4LCONTROL_SHM_SIZE);
-
-  /* Retreive a pointer to the shm object */
-  data->shm_values = mmap(NULL, V4LCONTROL_SHM_SIZE, (PROT_READ | PROT_WRITE),
-			  MAP_SHARED, shm_fd, 0);
-  close(shm_fd);
+  else
+    shm_fd = shm_open(shm_name, O_RDWR, (S_IREAD | S_IWRITE));
+  
+  if (shm_fd >= 0) {
+    /* Set the shared memory size */
+    ftruncate(shm_fd, V4LCONTROL_SHM_SIZE);
+
+    /* Retreive a pointer to the shm object */
+    data->shm_values = mmap(NULL, V4LCONTROL_SHM_SIZE, (PROT_READ | PROT_WRITE),
+			    MAP_SHARED, shm_fd, 0);
+    close(shm_fd);
+
+    if (data->shm_values == MAP_FAILED)
+      data->shm_values = NULL;
+  }
 
-  if (data->shm_values == MAP_FAILED)
-    goto error;
+  /* Fall back to malloc */
+  if (data->shm_values == NULL) {
+    data->shm_values = malloc(V4LCONTROL_SHM_SIZE);
+    if (!data->shm_values)
+      goto error;
+    
+    init = 1;
+    data->priv_flags |= V4LCONTROL_MEMORY_IS_MALLOCED;
+  }
 
   if (init) {
     /* Initialize the new shm object we created */
-    memset(data->shm_values, 0, sizeof(V4LCONTROL_SHM_SIZE));
+    memset(data->shm_values, 0, V4LCONTROL_SHM_SIZE);
 
     for (i = 0; i < V4LCONTROL_COUNT; i++)
       data->shm_values[i] = fake_controls[i].default_value;
@@ -359,8 +371,12 @@ error:
 
 void v4lcontrol_destroy(struct v4lcontrol_data *data)
 {
-  if (data->controls)
-    munmap(data->shm_values, V4LCONTROL_SHM_SIZE);
+  if (data->controls) {
+    if (data->priv_flags & V4LCONTROL_MEMORY_IS_MALLOCED)
+      free(data->shm_values);
+    else
+      munmap(data->shm_values, V4LCONTROL_SHM_SIZE);
+  }
   free(data);
 }