linux-kernel - Re: [PATCH v5] lightnvn: pblk

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]

Date:   Wed, 12 Apr 2017 00:13:29 +0200
From:   Javier González <jg@...htnvm.io>
To:     Bart Van Assche <Bart.VanAssche@...disk.com>
Cc:     Matias Bjørling <mb@...htnvm.io>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>
Subject: Re: [PATCH v5] lightnvn: pblk

Hi Bart,

> On 11 Apr 2017, at 17.19, Bart Van Assche <Bart.VanAssche@...disk.com> wrote:
> 
> On Tue, 2017-04-11 at 16:31 +0200, Javier González wrote:
>> Changes since v4:
>> * Rebase on top of Matias' for-4.12/core
>> * Fix type implicit conversions reported by sparse (reported by Bart Van
>>  Assche)
>> * Make error and debug statistics long atomic variables.
> 
> Hello Javier,
> 
> Thanks for the quick respin. But have you already had a look at the
> diagnostics reported by smatch? Smatch reports e.g.
> 
> drivers/lightnvm/pblk-rb.c:783: pblk_rb_tear_down_check() error: we previously assumed 'rb->entries' could be null (see line 779)
> 
> on the following code:
> 
> 	if (rb->entries)
> 		goto out;
> 
> 	for (i = 0; i < rb->nr_entries; i++) {
> 		entry = &rb->entries[i];
> 
> 		if (entry->data)
> 			goto out;
> 	}
> 
> Is that "if (rb->entries)" check correct or should that perhaps been
> "if (!rb->entries)"? Smatch is available at http://repo.or.cz/w/smatch.git.

I have run smatch over the code (did not know the tool, so thanks!).
This particular error has been fixed on v5. The only standing warning
relates to a semaphore on pblk-map that is taken on the erase path. This
is a false positive; it is intended that the semaphore lock is taken
here and then released on the completion path. Sparse and coccicheck
have been also been used on v5, but please point out to any other
tools/concerns you may have.

> Bart.

Thanks,

Javier

Download attachment "signature.asc" of type "application/pgp-signature" (802 bytes)