linux-kernel - Re: [PATCH 1/1] dm raid: fix compat

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] [day] [month] [year] [list]

Date:   Fri, 14 Oct 2016 19:14:32 +0200
From:   Heinz Mauelshagen <heinzm@...hat.com>
To:     Mike Snitzer <snitzer@...hat.com>
Cc:     Andy Whitcroft <apw@...onical.com>, linux-kernel@...r.kernel.org,
        linux-raid@...r.kernel.org, dm-devel@...hat.com,
        Shaohua Li <shli@...nel.org>, Alasdair Kergon <agk@...hat.com>
Subject: Re: [PATCH 1/1] dm raid: fix compat_features validation



On 10/11/2016 07:44 PM, Mike Snitzer wrote:
> On Tue, Oct 11 2016 at 11:44am -0400,
> Heinz Mauelshagen <heinzm@...hat.com> wrote:
>
>>
>> On 10/11/2016 05:38 PM, Andy Whitcroft wrote:
>>> On Tue, Oct 11, 2016 at 05:04:34PM +0200, Heinz Mauelshagen wrote:
>>>> Andy,
>>>>
>>>> good catch.
>>>>
>>>> We should rather check for  V190 support only in case any
>>>> compat feature flags are actually set.
>>>>
>>>> {
>>>> +       if (le32_to_cpu(sb->compat_features) &&
>>>> +           le32_to_cpu(sb->compat_features) != FEATURE_FLAG_SUPPORTS_V190)
>>>> {
>>>>                  rs->ti->error = "Unable to assemble array: Unknown flag(s)
>>>> in compatible feature flags";
>>>>                  return -EINVAL;
>>>>          }
>>> If the feature flags are single bit combinations then I believe the
>>> below does check exactly that.  Checking for no 1s outside of the
>>> expected features, caring not for the value of the valid bits:
>>>
>>> +     if (le32_to_cpu(sb->compat_features) & ~(FEATURE_FLAG_SUPPORTS_V190)) {
>>>
>>> with the possibilty to or in additional feature bits as they are added.
>> Thanks,
>> I prefer this to be easier readable.
> Readable or not, the code with the != is _not_ future-proof.  Whereas
> Andy's solution is.  If/when a new compat feature comes along then
> FEATURE_FLAG_SUPPORTS_V190 would be replaced to be a macro that ORs all
> the new compat features together (e.g. FEATURE_FLAG_COMPAT).  E.g. how
> dm-thin-metadata.c:__check_incompat_features() does.

If we'll have to introduce more feature flags in the future (e.g. for 
clustered raid1
support), this is going to be based on the test_bit() API for consistency
with any other flag processing we do in the target.

Heinz

> We can go with the != code for now, since any future changes would
> likely cause this test to be changed.  Or we could fix it now _for
> real_.
>
> Mike