linux-kernel - Re: [PATCH] lightnvm: missing nvm

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACygaLBOzyuWnBMbRguw2QNCOWGBam8BcGOEYj53mxjZgV4+Ag@mail.gmail.com>
Date:	Wed, 25 Nov 2015 21:11:39 +0800
From:	Wenwei Tao <ww.tao0320@...il.com>
To:	Matias Bjørling <mb@...htnvm.io>
Cc:	linux-kernel@...r.kernel.org, linux-block@...r.kernel.org
Subject: Re: [PATCH] lightnvm: missing nvm_lock acquire

Hi Matias

Place the lock inside nvm_find_nvm_dev can make code cleaner, but
sometime may lack of flexibility, cause redundant lock and unlock,
just my opinion. Place the lock inside the function is okay with me,
just make sure do the proper up_write(&nvm_lock) in
nvm_create_target(), it has a lot 'return'.

I just submit a patch set covering the lock issue and the target issue
mentioned before, you can have a look before you apply this patch.
Thanks.

2015-11-25 19:37 GMT+08:00 Matias Bjørling <mb@...htnvm.io>:
> On 11/25/2015 09:57 AM, Wenwei Tao wrote:
>>
>> Hi Matias
>> I think list_for_each_entry_safe(pos, n, head, member) cannot avoid
>> race condition
>> the item point by ‘n’ can be deleted and freed in the same time we
>> operate on 'pos'
>> so lock is still necessary.
>>
> I've made the patch like this, ok?
>
> To avoid race conditions, traverse dev, media manager,
> and target lists and also register, unregister entries
> to/from them, should be always under the nvm_lock control.
>
> Signed-off-by: Wenwei Tao <ww.tao0320@...il.com>
> Signed-off-by: Matias Bjørling <m@...rling.me>
> ---
>  drivers/lightnvm/core.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
> index ea6dba5..9f901f6 100644
> --- a/drivers/lightnvm/core.c
> +++ b/drivers/lightnvm/core.c
> @@ -127,9 +127,11 @@ static struct nvm_dev *nvm_find_nvm_dev(const char
> *name)
>  {
>         struct nvm_dev *dev;
>
> +       down_write(&nvm_lock);
>         list_for_each_entry(dev, &nvm_devices, devices)
>                 if (!strcmp(name, dev->name))
>                         return dev;
> +       up_write(&nvm_lock);
>
>         return NULL;
>  }
> @@ -388,13 +390,14 @@ static int nvm_create_target(struct nvm_dev *dev,
>                 }
>         }
>
> +       down_write(&nvm_lock);
>
>         tt = nvm_find_target_type(create->tgttype);
>         if (!tt) {
>                 pr_err("nvm: target type %s not found\n", create->tgttype);
> +               up_write(&nvm_lock);
>                 return -EINVAL;
>         }
>
> -       down_write(&nvm_lock);
>         list_for_each_entry(t, &dev->online_targets, list) {
>                 if (!strcmp(create->tgtname, t->disk->disk_name)) {
>                         pr_err("nvm: target name already exists.\n");
> --
> 2.1.4
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/