From 2c1ed6b34c4568ac8bfcd1ecd1b7bcef684fb98a Mon Sep 17 00:00:00 2001 From: Liangliang Lu Date: Fri, 20 Apr 2018 17:26:34 +0800 Subject: [PATCH] usb: gadget: ffs: Multi-instance fix for use after free case ffs can be used by many drivers, ADB can use it and MTP can use it. Any other generic function can use it. So we continue on the single instance patch, provide multi-instance fix for the use-after-free issue. Change-Id: I0056bd3779fb472b69e51391702b8b753d39372f Signed-off-by: Liangliang Lu --- drivers/usb/gadget/function/f_fs.c | 267 ++++++++++++++++++++--------- 1 file changed, 188 insertions(+), 79 deletions(-) diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c index 1ffde9c5408c..9edc01692142 100644 --- a/drivers/usb/gadget/function/f_fs.c +++ b/drivers/usb/gadget/function/f_fs.c @@ -68,18 +68,27 @@ __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); +static LIST_HEAD(inst_list); + /* ffs instance status */ -static DEFINE_MUTEX(ffs_ep_lock); -static bool ffs_inst_exist; -static struct f_fs_opts *g_opts; +#define INST_NAME_SIZE 16 + +struct ffs_inst_status { + char inst_name[INST_NAME_SIZE]; + struct list_head list; + struct mutex ffs_lock; + bool inst_exist; + struct f_fs_opts *opts; + struct ffs_data *ffs_data; +}; /* 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; +static void ffs_inst_clean(struct f_fs_opts *opts, + const char *inst_name); +static void ffs_inst_clean_delay(const char *inst_name); +static int ffs_inst_exist_check(const char *inst_name); +static struct ffs_inst_status *name_to_inst_status( + const char *inst_name, bool create_inst); /* The function structure ***************************************************/ @@ -300,7 +309,7 @@ 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(); + ret = ffs_inst_exist_check(ffs->dev_name); if (ret < 0) return ret; @@ -490,7 +499,7 @@ 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(); + ret = ffs_inst_exist_check(ffs->dev_name); if (ret < 0) return ret; @@ -601,7 +610,7 @@ static int ffs_ep0_open(struct inode *inode, struct file *file) 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(); + ret = ffs_inst_exist_check(ffs->dev_name); if (ret < 0) return ret; @@ -643,7 +652,7 @@ 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(); + ret = ffs_inst_exist_check(ffs->dev_name); if (ret < 0) return ret; @@ -668,7 +677,7 @@ 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(); + ret = ffs_inst_exist_check(ffs->dev_name); if (ret < 0) return ret; @@ -799,6 +808,10 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data) ffs_log("enter: epfile name %s epfile err %d (%s)", epfile->name, atomic_read(&epfile->error), io_data->read ? "READ" : "WRITE"); + ret = ffs_inst_exist_check(epfile->ffs->dev_name); + if (ret < 0) + return ret; + smp_mb__before_atomic(); retry: if (atomic_read(&epfile->error)) @@ -1085,7 +1098,7 @@ ffs_epfile_open(struct inode *inode, struct file *file) 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(); + ret = ffs_inst_exist_check(epfile->ffs->dev_name); if (ret < 0) return ret; @@ -1143,16 +1156,11 @@ 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)) @@ -1189,16 +1197,11 @@ 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)) @@ -1275,7 +1278,7 @@ 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(); + ret = ffs_inst_exist_check(epfile->ffs->dev_name); if (ret < 0) return ret; @@ -1583,6 +1586,7 @@ ffs_fs_mount(struct file_system_type *t, int flags, int ret; void *ffs_dev; struct ffs_data *ffs; + struct ffs_inst_status *inst_status; ENTER(); @@ -1612,6 +1616,18 @@ ffs_fs_mount(struct file_system_type *t, int flags, ffs->private_data = ffs_dev; data.ffs_data = ffs; + inst_status = name_to_inst_status(ffs->dev_name, false); + if (IS_ERR(inst_status)) { + ffs_log("failed to find instance (%s)\n", + ffs->dev_name); + return ERR_PTR(-EINVAL); + } + + /* Store ffs to global status structure */ + ffs_dev_lock(); + inst_status->ffs_data = ffs; + ffs_dev_unlock(); + rv = mount_nodev(t, flags, &data, ffs_sb_fill); if (IS_ERR(rv) && data.ffs_data) { ffs_release_dev(data.ffs_data); @@ -1711,6 +1727,9 @@ static void ffs_data_opened(struct ffs_data *ffs) static void ffs_data_put(struct ffs_data *ffs) { + struct ffs_inst_status *inst_status; + const char *dev_name; + ENTER(); ffs_log("enter"); @@ -1718,16 +1737,20 @@ 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(); + /* Clear ffs from global structure */ + inst_status = name_to_inst_status(ffs->dev_name, false); + if (!IS_ERR(inst_status)) { + ffs_dev_lock(); + inst_status->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); + dev_name = ffs->dev_name; kfree(ffs); - ffs_inst_clean_delay(); + ffs_inst_clean_delay(dev_name); + kfree(dev_name); } ffs_log("exit"); @@ -1792,11 +1815,6 @@ 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; @@ -3684,79 +3702,146 @@ static struct config_item_type ffs_func_type = { /* Function registration interface ******************************************/ -static int ffs_inst_exist_check(void) +static struct ffs_inst_status *name_to_inst_status( + const char *inst_name, bool create_inst) { - mutex_lock(&ffs_ep_lock); + struct ffs_inst_status *inst_status; - if (unlikely(ffs_inst_exist == false)) { - mutex_unlock(&ffs_ep_lock); + list_for_each_entry(inst_status, &inst_list, list) { + if (!strncasecmp(inst_status->inst_name, + inst_name, strlen(inst_name))) + return inst_status; + } + + if (!create_inst) + return ERR_PTR(-ENODEV); + + inst_status = kzalloc(sizeof(struct ffs_inst_status), + GFP_KERNEL); + if (!inst_status) + return ERR_PTR(-ENOMEM); + + mutex_init(&inst_status->ffs_lock); + snprintf(inst_status->inst_name, INST_NAME_SIZE, inst_name); + list_add_tail(&inst_status->list, &inst_list); + + return inst_status; +} + +static int ffs_inst_exist_check(const char *inst_name) +{ + struct ffs_inst_status *inst_status; + + inst_status = name_to_inst_status(inst_name, false); + if (IS_ERR(inst_status)) { pr_err_ratelimited( - "%s: f_fs instance freed already.\n", - __func__); + "%s: failed to find instance (%s)\n", + __func__, inst_name); return -ENODEV; } - mutex_unlock(&ffs_ep_lock); + mutex_lock(&inst_status->ffs_lock); + + if (unlikely(inst_status->inst_exist == false)) { + mutex_unlock(&inst_status->ffs_lock); + pr_err_ratelimited( + "%s: f_fs instance (%s) has been freed already.\n", + __func__, inst_name); + return -ENODEV; + } + + mutex_unlock(&inst_status->ffs_lock); return 0; } -static void ffs_inst_clean(struct f_fs_opts *opts) +static void ffs_inst_clean(struct f_fs_opts *opts, + const char *inst_name) { - g_opts = NULL; + struct ffs_inst_status *inst_status; + + inst_status = name_to_inst_status(inst_name, false); + if (IS_ERR(inst_status)) { + pr_err_ratelimited( + "%s: failed to find instance (%s)\n", + __func__, inst_name); + return; + } + + inst_status->opts = NULL; + ffs_dev_lock(); _ffs_free_dev(opts->dev); ffs_dev_unlock(); kfree(opts); } -static void ffs_inst_clean_delay(void) +static void ffs_inst_clean_delay(const char *inst_name) { - mutex_lock(&ffs_ep_lock); + struct ffs_inst_status *inst_status; - 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); + inst_status = name_to_inst_status(inst_name, false); + if (IS_ERR(inst_status)) { + pr_err_ratelimited( + "%s: failed to find (%s) instance\n", + __func__, inst_name); return; } - mutex_unlock(&ffs_ep_lock); + mutex_lock(&inst_status->ffs_lock); + + if (unlikely(inst_status->inst_exist == false)) { + if (inst_status->opts) { + ffs_inst_clean(inst_status->opts, inst_name); + pr_err_ratelimited("%s: Delayed free memory\n", + __func__); + } + mutex_unlock(&inst_status->ffs_lock); + return; + } + + mutex_unlock(&inst_status->ffs_lock); } static void ffs_free_inst(struct usb_function_instance *f) { struct f_fs_opts *opts; + struct ffs_inst_status *inst_status; 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__); + inst_status = name_to_inst_status(opts->dev->name, false); + if (IS_ERR(inst_status)) { + ffs_log("failed to find (%s) instance\n", + opts->dev->name); return; } - ffs_inst_clean(opts); - ffs_inst_exist = false; - g_opts = NULL; - mutex_unlock(&ffs_ep_lock); + mutex_lock(&inst_status->ffs_lock); + if (opts->dev->ffs_data + && atomic_read(&opts->dev->ffs_data->opened)) { + inst_status->inst_exist = false; + mutex_unlock(&inst_status->ffs_lock); + ffs_log("Dev is open, free mem when dev (%s) close\n", + opts->dev->name); + return; + } + + ffs_inst_clean(opts, opts->dev->name); + inst_status->inst_exist = false; + mutex_unlock(&inst_status->ffs_lock); } #define MAX_INST_NAME_LEN 40 static int ffs_set_inst_name(struct usb_function_instance *fi, const char *name) { - struct f_fs_opts *opts; + struct f_fs_opts *opts, *opts_prev; + struct ffs_data *ffs_data_tmp; char *ptr; const char *tmp; int name_len, ret; + struct ffs_inst_status *inst_status; name_len = strlen(name) + 1; if (name_len > MAX_INST_NAME_LEN) @@ -3766,13 +3851,22 @@ 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__); + inst_status = name_to_inst_status(ptr, true); + if (IS_ERR(inst_status)) { + ffs_log("failed to create status struct for (%s) instance\n", + ptr); + return -EINVAL; + } + + mutex_lock(&inst_status->ffs_lock); + opts_prev = inst_status->opts; + if (opts_prev) { + mutex_unlock(&inst_status->ffs_lock); + ffs_log("instance (%s): prev inst do not freed yet\n", + inst_status->inst_name); return -EBUSY; } - mutex_unlock(&ffs_ep_lock); + mutex_unlock(&inst_status->ffs_lock); opts = to_f_fs_opts(fi); tmp = NULL; @@ -3794,8 +3888,9 @@ static int ffs_set_inst_name(struct usb_function_instance *fi, const char *name) * ffs_private_data also need to update new allocated opts->dev * address. */ - if (g_ffs_data) - opts->dev->ffs_data = g_ffs_data; + ffs_data_tmp = inst_status->ffs_data; + if (ffs_data_tmp) + opts->dev->ffs_data = ffs_data_tmp; if (opts->dev->ffs_data) opts->dev->ffs_data->private_data = opts->dev; @@ -3804,10 +3899,10 @@ static int ffs_set_inst_name(struct usb_function_instance *fi, const char *name) kfree(tmp); - mutex_lock(&ffs_ep_lock); - ffs_inst_exist = true; - g_opts = opts; - mutex_unlock(&ffs_ep_lock); + mutex_lock(&inst_status->ffs_lock); + inst_status->inst_exist = true; + inst_status->opts = opts; + mutex_unlock(&inst_status->ffs_lock); return 0; } @@ -4212,6 +4307,20 @@ module_init(ffs_init); static void __exit ffs_exit(void) { + struct ffs_inst_status *inst_status, *inst_status_tmp = NULL; + + list_for_each_entry(inst_status, &inst_list, list) { + if (inst_status_tmp) { + list_del(&inst_status_tmp->list); + kfree(inst_status_tmp); + } + inst_status_tmp = inst_status; + } + if (inst_status_tmp) { + list_del(&inst_status_tmp->list); + kfree(inst_status_tmp); + } + if (ffs_ipc_log) { ipc_log_context_destroy(ffs_ipc_log); ffs_ipc_log = NULL;