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 <luliang@codeaurora.org>
This commit is contained in:
Liangliang Lu
parent
91da619c3a
commit
a17c88a808
1 changed files with 146 additions and 6 deletions
|
|
@ -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,15 +3682,69 @@ static struct config_item_type ffs_func_type = {
|
|||
|
||||
/* Function registration interface ******************************************/
|
||||
|
||||
static int ffs_inst_exist_check(void)
|
||||
{
|
||||
mutex_lock(&ffs_ep_lock);
|
||||
|
||||
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);
|
||||
ffs_dev_lock();
|
||||
_ffs_free_dev(opts->dev);
|
||||
ffs_dev_unlock();
|
||||
kfree(opts);
|
||||
|
||||
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
|
||||
|
|
@ -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;
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue