lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <57446273.70509@lightnvm.io> Date: Tue, 24 May 2016 16:17:23 +0200 From: Matias Bjørling <mb@...htnvm.io> To: Wenwei Tao <ww.tao0320@...il.com> Cc: Wenwei Tao <wwtao0320@....com>, linux-kernel@...r.kernel.org, linux-block@...r.kernel.org Subject: Re: [RFC PATCH 2/2] lightnvm: Append device name to target name On 05/23/2016 03:31 PM, Wenwei Tao wrote: > Eh.. my lock patch can only prevent concurrent creation of the same > name target on the same backend device, not the concurrent creation of > same name target on different backend devices, since target management > is protect by per device's gn->lock not > the global nvm_lock now. > > 2016-05-23 20:24 GMT+08:00 Matias Bjørling <mb@...htnvm.io>: >> On 05/23/2016 01:05 PM, Wenwei Tao wrote: >>> >>> 2016-05-23 17:16 GMT+08:00 Matias Bjørling <mb@...htnvm.io>: >>>> >>>> On 05/23/2016 11:13 AM, Wenwei Tao wrote: >>>>> >>>>> >>>>> From: Wenwei Tao <ww.tao0320@...il.com> >>>>> >>>>> We may create targets with same name on different >>>>> backend devices, this is not what we want, so append >>>>> the device name to target name to make the new target >>>>> name unique in the system. >>>>> >>>>> Signed-off-by: Wenwei Tao <ww.tao0320@...il.com> >>>>> --- >>>>> drivers/lightnvm/gennvm.c | 13 +++++++++++-- >>>>> 1 file changed, 11 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/drivers/lightnvm/gennvm.c b/drivers/lightnvm/gennvm.c >>>>> index 39ff0af..ecb09cb 100644 >>>>> --- a/drivers/lightnvm/gennvm.c >>>>> +++ b/drivers/lightnvm/gennvm.c >>>>> @@ -43,9 +43,18 @@ static int gen_create_tgt(struct nvm_dev *dev, struct >>>>> nvm_ioctl_create *create) >>>>> struct gendisk *tdisk; >>>>> struct nvm_tgt_type *tt; >>>>> struct nvm_target *t; >>>>> + char tgtname[DISK_NAME_LEN]; >>>>> void *targetdata; >>>>> int ret = -ENOMEM; >>>>> >>>>> + if (strlen(dev->name) + strlen(create->tgtname) + 1 > >>>>> DISK_NAME_LEN) { >>>>> + pr_err("nvm: target name too long. %s:%s\n", >>>>> + dev->name, create->tgtname); >>>>> + return -EINVAL; >>>>> + } >>>>> + >>>>> + sprintf(tgtname, "%s%s", dev->name, create->tgtname); >>>>> + >>>>> tt = nvm_find_target_type(create->tgttype, 1); >>>>> if (!tt) { >>>>> pr_err("nvm: target type %s not found\n", >>>>> create->tgttype); >>>>> @@ -53,7 +62,7 @@ static int gen_create_tgt(struct nvm_dev *dev, struct >>>>> nvm_ioctl_create *create) >>>>> } >>>>> >>>>> mutex_lock(&gn->lock); >>>>> - t = gen_find_target(gn, create->tgtname); >>>>> + t = gen_find_target(gn, tgtname); >>>>> if (t) { >>>>> pr_err("nvm: target name already exists.\n"); >>>>> ret = -EINVAL; >>>>> @@ -73,7 +82,7 @@ static int gen_create_tgt(struct nvm_dev *dev, struct >>>>> nvm_ioctl_create *create) >>>>> if (!tdisk) >>>>> goto err_queue; >>>>> >>>>> - sprintf(tdisk->disk_name, "%s", create->tgtname); >>>>> + sprintf(tdisk->disk_name, "%s", tgtname); >>>>> tdisk->flags = GENHD_FL_EXT_DEVT; >>>>> tdisk->major = 0; >>>>> tdisk->first_minor = 0; >>>>> >>>> >>>> Hi Wenwei, what about the case where a target instance has multiple >>>> devices >>>> associated? >>>> >>> You mean a target may be build on multiple backend devices ? >> >> >> Yes. Over time, we want a single target to manage many devices. >> >>> >>>> I am okay with having the user choosing a unique name for the target to >>>> be >>>> exposed. >>> >>> You mean user should check the name before create the target? >> >> >> Sure. It is him that decides the name of the device. Your lock patch fixes >> the panic that could happen. I am happy with that. >> >> >>> Before move target mgmt into media mgr, that would be okay(after apply >>> lightnvm: hold lock until finish the target creation), since all >>> targets are in the global list. >>> Now consider below case: >>> There are two users A and B. A want to create target test0 upon >>> device0 B want to create test0 upon device1, >>> before creation they both check whether test0 is exist (e.g. by list >>> /dev/test0) , they all find test0 is not exist now, and they continue >>> their >>> creation. Both of them use disk name test0 to call add_disk, that >>> would cause panic. >>> >> You are right. The right way is properly to not allow the user to define a device name, and instead rely on generic naming and UUID.
Powered by blists - more mailing lists