From a17c88a8083977fb552de7ec78b8bc1ddf554465 Mon Sep 17 00:00:00 2001 From: Liangliang Lu Date: Tue, 7 Nov 2017 18:53:08 +0800 Subject: usb: gadget: ffs: Defer freeing memory on free_inst if in use In the case of ffs_free_inst() called, whole ffs_dev structure is freed. Userspace related API do not check if ffs_dev is freed or not. If ffs endpoint is opened by userspace, ffs_free_inst() is executed, mark inst_exist to false but do not free instance structures until ffs_data is freed. Besides, ffs_data is allocated in ffs_fs_mount() while opts->dev is allocated when ffs instance created. And opts->dev will be freed when ffs instance freed. If ffs instance is freed and created once, opts->dev is allocated to new memory, but since ffs_fs_mount() won't be called in this case, new opts->dev miss the ffs_data address and ffs_data->private_data still point to old opts->dev address which is already freed. So new allocated opts->dev need to initialize opts->dev->ffs_data, and ffs_private_data also need to update new allocated opts->dev address. Change-Id: Idea56f86c62da700926e8ce3a724d5be6295a4fd Signed-off-by: Liangliang Lu --- drivers/usb/gadget/function/f_fs.c | 150 +++++++++++++++++++++++++++++++++++-- 1 file changed, 145 insertions(+), 5 deletions(-) (limited to 'drivers/usb') diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c index bb02b3be17f7..2cbd90a7db43 100644 --- a/drivers/usb/gadget/function/f_fs.c +++ b/drivers/usb/gadget/function/f_fs.c @@ -49,7 +49,7 @@ static void *ffs_ipc_log; if (ffs_ipc_log) \ ipc_log_string(ffs_ipc_log, "%s: " fmt, __func__, \ ##__VA_ARGS__); \ - pr_debug(fmt, ##__VA_ARGS__); \ + pr_debug("%s: " fmt, __func__, ##__VA_ARGS__); \ } while (0) /* Reference counter handling */ @@ -68,6 +68,18 @@ __ffs_data_got_descs(struct ffs_data *ffs, char *data, size_t len); static int __must_check __ffs_data_got_strings(struct ffs_data *ffs, char *data, size_t len); +/* ffs instance status */ +static DEFINE_MUTEX(ffs_ep_lock); +static bool ffs_inst_exist; +static struct f_fs_opts *g_opts; + +/* Free instance structures */ +static void ffs_inst_clean(struct f_fs_opts *opts); +static void ffs_inst_clean_delay(void); +static int ffs_inst_exist_check(void); + +/* Global ffs_data pointer */ +static struct ffs_data *g_ffs_data; /* The function structure ***************************************************/ @@ -288,6 +300,10 @@ static ssize_t ffs_ep0_write(struct file *file, const char __user *buf, ffs_log("enter:len %zu state %d setup_state %d flags %lu", len, ffs->state, ffs->setup_state, ffs->flags); + ret = ffs_inst_exist_check(); + if (ret < 0) + return ret; + /* Fast check if setup was canceled */ if (ffs_setup_state_clear_cancelled(ffs) == FFS_SETUP_CANCELLED) return -EIDRM; @@ -474,6 +490,10 @@ static ssize_t ffs_ep0_read(struct file *file, char __user *buf, ffs_log("enter:len %zu state %d setup_state %d flags %lu", len, ffs->state, ffs->setup_state, ffs->flags); + ret = ffs_inst_exist_check(); + if (ret < 0) + return ret; + /* Fast check if setup was canceled */ if (ffs_setup_state_clear_cancelled(ffs) == FFS_SETUP_CANCELLED) return -EIDRM; @@ -574,12 +594,17 @@ done_mutex: static int ffs_ep0_open(struct inode *inode, struct file *file) { struct ffs_data *ffs = inode->i_private; + int ret; ENTER(); ffs_log("state %d setup_state %d flags %lu opened %d", ffs->state, ffs->setup_state, ffs->flags, atomic_read(&ffs->opened)); + ret = ffs_inst_exist_check(); + if (ret < 0) + return ret; + if (unlikely(ffs->state == FFS_CLOSING)) return -EBUSY; @@ -618,6 +643,10 @@ static long ffs_ep0_ioctl(struct file *file, unsigned code, unsigned long value) ffs_log("state %d setup_state %d flags %lu opened %d", ffs->state, ffs->setup_state, ffs->flags, atomic_read(&ffs->opened)); + ret = ffs_inst_exist_check(); + if (ret < 0) + return ret; + if (code == FUNCTIONFS_INTERFACE_REVMAP) { struct ffs_function *func = ffs->func; ret = func ? ffs_func_revmap_intf(func, value) : -ENODEV; @@ -639,6 +668,10 @@ static unsigned int ffs_ep0_poll(struct file *file, poll_table *wait) ffs_log("enter:state %d setup_state %d flags %lu opened %d", ffs->state, ffs->setup_state, ffs->flags, atomic_read(&ffs->opened)); + ret = ffs_inst_exist_check(); + if (ret < 0) + return ret; + poll_wait(file, &ffs->ev.waitq, wait); ret = ffs_mutex_lock(&ffs->mutex, file->f_flags & O_NONBLOCK); @@ -1045,12 +1078,17 @@ static int ffs_epfile_open(struct inode *inode, struct file *file) { struct ffs_epfile *epfile = inode->i_private; + int ret; ENTER(); ffs_log("enter:state %d setup_state %d flag %lu", epfile->ffs->state, epfile->ffs->setup_state, epfile->ffs->flags); + ret = ffs_inst_exist_check(); + if (ret < 0) + return ret; + if (WARN_ON(epfile->ffs->state != FFS_ACTIVE)) return -ENODEV; @@ -1105,11 +1143,16 @@ static ssize_t ffs_epfile_write_iter(struct kiocb *kiocb, struct iov_iter *from) { struct ffs_io_data io_data, *p = &io_data; ssize_t res; + int ret; ENTER(); ffs_log("enter"); + ret = ffs_inst_exist_check(); + if (ret < 0) + return ret; + if (!is_sync_kiocb(kiocb)) { p = kmalloc(sizeof(io_data), GFP_KERNEL); if (unlikely(!p)) @@ -1146,11 +1189,16 @@ static ssize_t ffs_epfile_read_iter(struct kiocb *kiocb, struct iov_iter *to) { struct ffs_io_data io_data, *p = &io_data; ssize_t res; + int ret; ENTER(); ffs_log("enter"); + ret = ffs_inst_exist_check(); + if (ret < 0) + return ret; + if (!is_sync_kiocb(kiocb)) { p = kmalloc(sizeof(io_data), GFP_KERNEL); if (unlikely(!p)) @@ -1227,6 +1275,10 @@ static long ffs_epfile_ioctl(struct file *file, unsigned code, ffs_log("enter:state %d setup_state %d flag %lu", epfile->ffs->state, epfile->ffs->setup_state, epfile->ffs->flags); + ret = ffs_inst_exist_check(); + if (ret < 0) + return ret; + if (WARN_ON(epfile->ffs->state != FFS_ACTIVE)) return -ENODEV; @@ -1582,7 +1634,6 @@ ffs_fs_kill_sb(struct super_block *sb) if (sb->s_fs_info) { ffs_release_dev(sb->s_fs_info); ffs_data_closed(sb->s_fs_info); - ffs_data_put(sb->s_fs_info); } ffs_log("exit"); @@ -1667,11 +1718,16 @@ static void ffs_data_put(struct ffs_data *ffs) smp_mb__before_atomic(); if (unlikely(atomic_dec_and_test(&ffs->ref))) { pr_info("%s(): freeing\n", __func__); + /* Clear g_ffs_data */ + ffs_dev_lock(); + g_ffs_data = NULL; + ffs_dev_unlock(); ffs_data_clear(ffs); BUG_ON(waitqueue_active(&ffs->ev.waitq) || waitqueue_active(&ffs->ep0req_completion.wait)); kfree(ffs->dev_name); kfree(ffs); + ffs_inst_clean_delay(); } ffs_log("exit"); @@ -1736,6 +1792,11 @@ static struct ffs_data *ffs_data_new(void) /* XXX REVISIT need to update it in some places, or do we? */ ffs->ev.can_stall = 1; + /* Store ffs to g_ffs_data */ + ffs_dev_lock(); + g_ffs_data = ffs; + ffs_dev_unlock(); + ffs_log("exit"); return ffs; @@ -3621,17 +3682,71 @@ static struct config_item_type ffs_func_type = { /* Function registration interface ******************************************/ -static void ffs_free_inst(struct usb_function_instance *f) +static int ffs_inst_exist_check(void) { - struct f_fs_opts *opts; + mutex_lock(&ffs_ep_lock); - opts = to_f_fs_opts(f); + if (unlikely(ffs_inst_exist == false)) { + mutex_unlock(&ffs_ep_lock); + pr_err_ratelimited( + "%s: f_fs instance freed already.\n", + __func__); + return -ENODEV; + } + + mutex_unlock(&ffs_ep_lock); + + return 0; +} + +static void ffs_inst_clean(struct f_fs_opts *opts) +{ + g_opts = NULL; ffs_dev_lock(); _ffs_free_dev(opts->dev); ffs_dev_unlock(); kfree(opts); } +static void ffs_inst_clean_delay(void) +{ + mutex_lock(&ffs_ep_lock); + + if (unlikely(ffs_inst_exist == false)) { + if (g_opts) { + ffs_inst_clean(g_opts); + pr_err_ratelimited("%s: Delayed free memory\n", + __func__); + } + mutex_unlock(&ffs_ep_lock); + return; + } + + mutex_unlock(&ffs_ep_lock); +} + +static void ffs_free_inst(struct usb_function_instance *f) +{ + struct f_fs_opts *opts; + + opts = to_f_fs_opts(f); + + mutex_lock(&ffs_ep_lock); + if (opts->dev->ffs_data + && atomic_read(&opts->dev->ffs_data->opened)) { + ffs_inst_exist = false; + mutex_unlock(&ffs_ep_lock); + ffs_log("%s: Dev is open, free mem when dev close\n", + __func__); + return; + } + + ffs_inst_clean(opts); + ffs_inst_exist = false; + g_opts = NULL; + mutex_unlock(&ffs_ep_lock); +} + #define MAX_INST_NAME_LEN 40 static int ffs_set_inst_name(struct usb_function_instance *fi, const char *name) @@ -3649,6 +3764,14 @@ static int ffs_set_inst_name(struct usb_function_instance *fi, const char *name) if (!ptr) return -ENOMEM; + mutex_lock(&ffs_ep_lock); + if (g_opts) { + mutex_unlock(&ffs_ep_lock); + ffs_log("%s: prev inst do not freed yet\n", __func__); + return -EBUSY; + } + mutex_unlock(&ffs_ep_lock); + opts = to_f_fs_opts(fi); tmp = NULL; @@ -3663,10 +3786,27 @@ static int ffs_set_inst_name(struct usb_function_instance *fi, const char *name) } opts->dev->name_allocated = true; + /* + * If ffs instance is freed and created once, new allocated + * opts->dev need to initialize opts->dev->ffs_data, and + * ffs_private_data also need to update new allocated opts->dev + * address. + */ + if (g_ffs_data) + opts->dev->ffs_data = g_ffs_data; + + if (opts->dev->ffs_data) + opts->dev->ffs_data->private_data = opts->dev; + ffs_dev_unlock(); kfree(tmp); + mutex_lock(&ffs_ep_lock); + ffs_inst_exist = true; + g_opts = opts; + mutex_unlock(&ffs_ep_lock); + return 0; } -- cgit v1.2.3