]> git.hungrycats.org Git - linux/commitdiff
VCHIQ: Fix mem leak of USER_SERVICE_T objects.
authorMatthew Hails <mhails@broadcom.com>
Mon, 13 May 2013 13:22:48 +0000 (14:22 +0100)
committerpopcornmix <popcornmix@gmail.com>
Thu, 1 Aug 2013 11:30:28 +0000 (12:30 +0100)
The userdata for VCHIQ services created through the ioctl API is
a kmalloced structure. These objects were getting leaked, most
notably in vchiq_release(), where the service could be closed,
freed and removed from the service list before the wait-to-die
loop was entered.

This change adds a userdata termination callback, and implements
it in the case where USER_SERVICE_T is used for the service
userdata.

drivers/misc/vc04_services/interface/vchiq_arm/vchiq_arm.c
drivers/misc/vc04_services/interface/vchiq_arm/vchiq_core.c
drivers/misc/vc04_services/interface/vchiq_arm/vchiq_core.h
drivers/misc/vc04_services/interface/vchiq_arm/vchiq_kern_lib.c

index 7298d8519e9eb6d2ef90523f45c4e95ede0cdf16..c062a95a675f0b50b136f0b477dcc80b102ad905 100644 (file)
@@ -357,6 +357,17 @@ service_callback(VCHIQ_REASON_T reason, VCHIQ_HEADER_T *header,
                bulk_userdata);
 }
 
+/****************************************************************************
+*
+*   user_service_free
+*
+***************************************************************************/
+static void
+user_service_free(void *userdata)
+{
+       kfree(userdata);
+}
+
 /****************************************************************************
 *
 *   vchiq_ioctl
@@ -467,7 +478,7 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
                service = vchiq_add_service_internal(
                                instance->state,
                                &args.params, srvstate,
-                               instance);
+                               instance, user_service_free);
 
                if (service != NULL) {
                        user_service->service = service;
@@ -490,8 +501,6 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
                                        service = NULL;
                                        ret = (status == VCHIQ_RETRY) ?
                                                -EINTR : -EIO;
-                                       user_service->service = NULL;
-                                       user_service->instance = NULL;
                                        break;
                                }
                        }
@@ -503,7 +512,6 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
                                sizeof(service->handle)) != 0) {
                                ret = -EFAULT;
                                vchiq_remove_service(service->handle);
-                               kfree(user_service);
                        }
 
                        service = NULL;
@@ -796,10 +804,8 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
                                }
 
                                if (completion->reason ==
-                                       VCHIQ_SERVICE_CLOSED) {
+                                       VCHIQ_SERVICE_CLOSED)
                                        unlock_service(service);
-                                       kfree(user_service);
-                               }
 
                                if (copy_to_user((void __user *)(
                                        (size_t)args.buf +
@@ -1151,7 +1157,6 @@ vchiq_release(struct inode *inode, struct file *file)
                        spin_unlock(&msg_queue_spinlock);
 
                        unlock_service(service);
-                       kfree(user_service);
                }
 
                /* Release any closed services */
index 2efb12411a6058f20d30ee33d46994ca21b274d0..9f66704f940deda9d92d1ed61a81c48a2c35eda0 100644 (file)
@@ -273,6 +273,9 @@ unlock_service(VCHIQ_SERVICE_T *service)
        }
        spin_unlock(&service_spinlock);
 
+       if (service && service->userdata_term)
+               service->userdata_term(service->base.userdata);
+
        kfree(service);
 }
 
@@ -2476,7 +2479,7 @@ vchiq_init_state(VCHIQ_STATE_T *state, VCHIQ_SLOT_ZERO_T *slot_zero,
 VCHIQ_SERVICE_T *
 vchiq_add_service_internal(VCHIQ_STATE_T *state,
        const VCHIQ_SERVICE_PARAMS_T *params, int srvstate,
-       VCHIQ_INSTANCE_T instance)
+       VCHIQ_INSTANCE_T instance, VCHIQ_USERDATA_TERM_T userdata_term)
 {
        VCHIQ_SERVICE_T *service;
 
@@ -2488,6 +2491,7 @@ vchiq_add_service_internal(VCHIQ_STATE_T *state,
                service->handle        = VCHIQ_SERVICE_HANDLE_INVALID;
                service->ref_count     = 1;
                service->srvstate      = VCHIQ_SRVSTATE_FREE;
+               service->userdata_term = userdata_term;
                service->localport     = VCHIQ_PORT_FREE;
                service->remoteport    = VCHIQ_PORT_FREE;
 
index bddfc6a19ee728c8ad274b88c5bad831dcd00519..47cdf2775fe182df980acfc5ec27ded5603002c4 100644 (file)
@@ -237,6 +237,8 @@ typedef enum {
        VCHIQ_BULK_RECEIVE
 } VCHIQ_BULK_DIR_T;
 
+typedef void (*VCHIQ_USERDATA_TERM_T)(void *userdata);
+
 typedef struct vchiq_bulk_struct {
        short mode;
        short dir;
@@ -284,6 +286,7 @@ typedef struct vchiq_service_struct {
        VCHIQ_SERVICE_HANDLE_T handle;
        unsigned int ref_count;
        int srvstate;
+       VCHIQ_USERDATA_TERM_T userdata_term;
        unsigned int localport;
        unsigned int remoteport;
        int public_fourcc;
@@ -534,7 +537,7 @@ vchiq_connect_internal(VCHIQ_STATE_T *state, VCHIQ_INSTANCE_T instance);
 extern VCHIQ_SERVICE_T *
 vchiq_add_service_internal(VCHIQ_STATE_T *state,
        const VCHIQ_SERVICE_PARAMS_T *params, int srvstate,
-       VCHIQ_INSTANCE_T instance);
+       VCHIQ_INSTANCE_T instance, VCHIQ_USERDATA_TERM_T userdata_term);
 
 extern VCHIQ_STATUS_T
 vchiq_open_service_internal(VCHIQ_SERVICE_T *service, int client_id);
index 62965c65d2b5660f38e01cfa5bbae44dc152daec..be9735f214d7ab435670c9a0d1b85e49a8ffdb19 100644 (file)
@@ -241,7 +241,8 @@ VCHIQ_STATUS_T vchiq_add_service(
                state,
                params,
                srvstate,
-               instance);
+               instance,
+               NULL);
 
        if (service) {
                *phandle = service->handle;
@@ -282,7 +283,8 @@ VCHIQ_STATUS_T vchiq_open_service(
        service = vchiq_add_service_internal(state,
                params,
                VCHIQ_SRVSTATE_OPENING,
-               instance);
+               instance,
+               NULL);
 
        if (service) {
                status = vchiq_open_service_internal(service, current->pid);