Commit 9f5aa2a9 authored by Patrick Caulfield's avatar Patrick Caulfield Committed by Steven Whitehouse

[DLM] Fix potential conflict in DLM userland locks

Just spotted this one. The lockinfo structs are hashed by lockid but into a
global structure. So that if there are two lockspaces with the same lockid all
hell breaks loose. I'm not exactly sure what will happen but it can't be good!

The attached patch moves the lockinfo_idr into the user_ls structure so that
lockids are localised.

patrick
Signed-Off-By: default avatarPatrick Caulfield <pcaulfie@redhat.com>
Signed-off-by: default avatarSteven Whitehouse <swhiteho@redhat.com>
parent b61dde79
...@@ -45,10 +45,6 @@ static const char *name_prefix="dlm"; ...@@ -45,10 +45,6 @@ static const char *name_prefix="dlm";
static struct list_head user_ls_list; static struct list_head user_ls_list;
static struct mutex user_ls_lock; static struct mutex user_ls_lock;
/* Lock infos are stored in here indexed by lock ID */
static DEFINE_IDR(lockinfo_idr);
static rwlock_t lockinfo_lock;
/* Flags in li_flags */ /* Flags in li_flags */
#define LI_FLAG_COMPLETE 1 #define LI_FLAG_COMPLETE 1
#define LI_FLAG_FIRSTLOCK 2 #define LI_FLAG_FIRSTLOCK 2
...@@ -99,6 +95,10 @@ struct user_ls { ...@@ -99,6 +95,10 @@ struct user_ls {
atomic_t ls_refcnt; atomic_t ls_refcnt;
long ls_flags; long ls_flags;
/* Lock infos are stored in here indexed by lock ID */
struct idr lockinfo_idr;
rwlock_t lockinfo_lock;
/* Passed into misc_register() */ /* Passed into misc_register() */
struct miscdevice ls_miscinfo; struct miscdevice ls_miscinfo;
struct list_head ls_list; struct list_head ls_list;
...@@ -240,13 +240,13 @@ static void put_file_info(struct file_info *f) ...@@ -240,13 +240,13 @@ static void put_file_info(struct file_info *f)
kfree(f); kfree(f);
} }
static void release_lockinfo(struct lock_info *li) static void release_lockinfo(struct user_ls *ls, struct lock_info *li)
{ {
put_file_info(li->li_file); put_file_info(li->li_file);
write_lock(&lockinfo_lock); write_lock(&ls->lockinfo_lock);
idr_remove(&lockinfo_idr, li->li_lksb.sb_lkid); idr_remove(&ls->lockinfo_idr, li->li_lksb.sb_lkid);
write_unlock(&lockinfo_lock); write_unlock(&ls->lockinfo_lock);
if (li->li_lksb.sb_lvbptr) if (li->li_lksb.sb_lvbptr)
kfree(li->li_lksb.sb_lvbptr); kfree(li->li_lksb.sb_lvbptr);
...@@ -255,46 +255,46 @@ static void release_lockinfo(struct lock_info *li) ...@@ -255,46 +255,46 @@ static void release_lockinfo(struct lock_info *li)
module_put(THIS_MODULE); module_put(THIS_MODULE);
} }
static struct lock_info *get_lockinfo(uint32_t lockid) static struct lock_info *get_lockinfo(struct user_ls *ls, uint32_t lockid)
{ {
struct lock_info *li; struct lock_info *li;
read_lock(&lockinfo_lock); read_lock(&ls->lockinfo_lock);
li = idr_find(&lockinfo_idr, lockid); li = idr_find(&ls->lockinfo_idr, lockid);
read_unlock(&lockinfo_lock); read_unlock(&ls->lockinfo_lock);
return li; return li;
} }
static int add_lockinfo(struct lock_info *li) static int add_lockinfo(struct user_ls *ls, struct lock_info *li)
{ {
int n; int n;
int r; int r;
int ret = -EINVAL; int ret = -EINVAL;
write_lock(&lockinfo_lock); write_lock(&ls->lockinfo_lock);
if (idr_find(&lockinfo_idr, li->li_lksb.sb_lkid)) if (idr_find(&ls->lockinfo_idr, li->li_lksb.sb_lkid))
goto out_up; goto out_up;
ret = -ENOMEM; ret = -ENOMEM;
r = idr_pre_get(&lockinfo_idr, GFP_KERNEL); r = idr_pre_get(&ls->lockinfo_idr, GFP_KERNEL);
if (!r) if (!r)
goto out_up; goto out_up;
r = idr_get_new_above(&lockinfo_idr, li, li->li_lksb.sb_lkid, &n); r = idr_get_new_above(&ls->lockinfo_idr, li, li->li_lksb.sb_lkid, &n);
if (r) if (r)
goto out_up; goto out_up;
if (n != li->li_lksb.sb_lkid) { if (n != li->li_lksb.sb_lkid) {
idr_remove(&lockinfo_idr, n); idr_remove(&ls->lockinfo_idr, n);
goto out_up; goto out_up;
} }
ret = 0; ret = 0;
out_up: out_up:
write_unlock(&lockinfo_lock); write_unlock(&ls->lockinfo_lock);
return ret; return ret;
} }
...@@ -358,6 +358,9 @@ static int register_lockspace(char *name, struct user_ls **ls, int flags) ...@@ -358,6 +358,9 @@ static int register_lockspace(char *name, struct user_ls **ls, int flags)
return status; return status;
} }
idr_init(&newls->lockinfo_idr);
rwlock_init(&newls->lockinfo_lock);
snprintf((char*)newls->ls_miscinfo.name, namelen, "%s_%s", snprintf((char*)newls->ls_miscinfo.name, namelen, "%s_%s",
name_prefix, name); name_prefix, name);
...@@ -486,13 +489,13 @@ static void ast_routine(void *param) ...@@ -486,13 +489,13 @@ static void ast_routine(void *param)
list_del(&li->li_ownerqueue); list_del(&li->li_ownerqueue);
clear_bit(LI_FLAG_ONLIST, &li->li_flags); clear_bit(LI_FLAG_ONLIST, &li->li_flags);
spin_unlock(&li->li_file->fi_li_lock); spin_unlock(&li->li_file->fi_li_lock);
release_lockinfo(li); release_lockinfo(li->li_file->fi_ls, li);
return; return;
} }
/* Free unlocks & queries */ /* Free unlocks & queries */
if (li->li_lksb.sb_status == -DLM_EUNLOCK || if (li->li_lksb.sb_status == -DLM_EUNLOCK ||
li->li_cmd == DLM_USER_QUERY) { li->li_cmd == DLM_USER_QUERY) {
release_lockinfo(li); release_lockinfo(li->li_file->fi_ls, li);
} }
} else { } else {
/* Synchronous request, just wake up the caller */ /* Synchronous request, just wake up the caller */
...@@ -641,7 +644,7 @@ static int dlm_close(struct inode *inode, struct file *file) ...@@ -641,7 +644,7 @@ static int dlm_close(struct inode *inode, struct file *file)
old_li->li_lksb.sb_lkid, status); old_li->li_lksb.sb_lkid, status);
/* But tidy our references in it */ /* But tidy our references in it */
release_lockinfo(old_li); release_lockinfo(old_li->li_file->fi_ls, old_li);
continue; continue;
} }
...@@ -662,7 +665,7 @@ static int dlm_close(struct inode *inode, struct file *file) ...@@ -662,7 +665,7 @@ static int dlm_close(struct inode *inode, struct file *file)
/* Unlock suceeded, free the lock_info struct. */ /* Unlock suceeded, free the lock_info struct. */
if (status == 0) if (status == 0)
release_lockinfo(old_li); release_lockinfo(old_li->li_file->fi_ls, old_li);
} }
remove_wait_queue(&li.li_waitq, &wq); remove_wait_queue(&li.li_waitq, &wq);
...@@ -920,7 +923,7 @@ static int do_user_lock(struct file_info *fi, uint8_t cmd, ...@@ -920,7 +923,7 @@ static int do_user_lock(struct file_info *fi, uint8_t cmd,
unless we are adopting an orphaned persistent lock */ unless we are adopting an orphaned persistent lock */
if (kparams->flags & DLM_LKF_CONVERT) { if (kparams->flags & DLM_LKF_CONVERT) {
li = get_lockinfo(kparams->lkid); li = get_lockinfo(fi->fi_ls, kparams->lkid);
/* If this is a persistent lock we will have to create a /* If this is a persistent lock we will have to create a
lockinfo again */ lockinfo again */
...@@ -938,7 +941,7 @@ static int do_user_lock(struct file_info *fi, uint8_t cmd, ...@@ -938,7 +941,7 @@ static int do_user_lock(struct file_info *fi, uint8_t cmd,
fail we want rid of it */ fail we want rid of it */
init_completion(&li->li_firstcomp); init_completion(&li->li_firstcomp);
set_bit(LI_FLAG_FIRSTLOCK, &li->li_flags); set_bit(LI_FLAG_FIRSTLOCK, &li->li_flags);
add_lockinfo(li); add_lockinfo(fi->fi_ls, li);
/* TODO: do a query to get the current state ?? */ /* TODO: do a query to get the current state ?? */
} }
...@@ -1014,7 +1017,7 @@ static int do_user_lock(struct file_info *fi, uint8_t cmd, ...@@ -1014,7 +1017,7 @@ static int do_user_lock(struct file_info *fi, uint8_t cmd,
list_add(&li->li_ownerqueue, &fi->fi_li_list); list_add(&li->li_ownerqueue, &fi->fi_li_list);
set_bit(LI_FLAG_ONLIST, &li->li_flags); set_bit(LI_FLAG_ONLIST, &li->li_flags);
spin_unlock(&fi->fi_li_lock); spin_unlock(&fi->fi_li_lock);
if (add_lockinfo(li)) if (add_lockinfo(fi->fi_ls, li))
printk(KERN_WARNING "Add lockinfo failed\n"); printk(KERN_WARNING "Add lockinfo failed\n");
complete(&li->li_firstcomp); complete(&li->li_firstcomp);
...@@ -1025,7 +1028,7 @@ static int do_user_lock(struct file_info *fi, uint8_t cmd, ...@@ -1025,7 +1028,7 @@ static int do_user_lock(struct file_info *fi, uint8_t cmd,
out_err: out_err:
if (test_bit(LI_FLAG_FIRSTLOCK, &li->li_flags)) if (test_bit(LI_FLAG_FIRSTLOCK, &li->li_flags))
release_lockinfo(li); release_lockinfo(fi->fi_ls, li);
return status; return status;
} }
...@@ -1037,7 +1040,7 @@ static int do_user_unlock(struct file_info *fi, uint8_t cmd, ...@@ -1037,7 +1040,7 @@ static int do_user_unlock(struct file_info *fi, uint8_t cmd,
int status; int status;
int convert_cancel = 0; int convert_cancel = 0;
li = get_lockinfo(kparams->lkid); li = get_lockinfo(fi->fi_ls, kparams->lkid);
if (!li) { if (!li) {
li = allocate_lockinfo(fi, cmd, kparams); li = allocate_lockinfo(fi, cmd, kparams);
if (!li) if (!li)
...@@ -1209,7 +1212,6 @@ static int __init dlm_device_init(void) ...@@ -1209,7 +1212,6 @@ static int __init dlm_device_init(void)
INIT_LIST_HEAD(&user_ls_list); INIT_LIST_HEAD(&user_ls_list);
mutex_init(&user_ls_lock); mutex_init(&user_ls_lock);
rwlock_init(&lockinfo_lock);
ctl_device.name = "dlm-control"; ctl_device.name = "dlm-control";
ctl_device.fops = &_dlm_ctl_fops; ctl_device.fops = &_dlm_ctl_fops;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment