linux-ext4 - Re: Patch adding e2p_feature_to

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: <CAC80aejg3USDDeaVQ-F72aWoAk1daqdpTRCOiOac=GjVsqj6Pg@mail.gmail.com>
Date:	Thu, 18 Dec 2014 10:23:15 -0800
From:	👾 Wade <wadetregaskis@...gle.com>
To:	Andreas Dilger <adilger@...ger.ca>
Cc:	linux-ext4@...r.kernel.org
Subject: Re: Patch adding e2p_feature_to_string

> I see that now.  That said, your version now does this for _every_ caller
> of e2p_feature2string(), while the original code only did it in the rare
> case of an unsupported feature bit set on on a filesystem.  I don't see
> that as an improvement over the original.

I do - the existing code is causing data corruption in my use.

I'm unsure why the string copy is problematic - this doesn't seem to
be performance-sensitive code.  As far as I could find, all uses of
this function in the e2fsprogs codebase are just printfing the result,
in human-readable format.  Or for debugging.  ?

> One option, maybe not very pretty but not fatally ugly, is to just
> initialize all of the "unknown" features as static strings.  That ends
> up being 23 extra "FEATURE_Cnn" strings, 21 extra "FEATURE_Rnn" strings,
> and 17 extra "FEATURE_Inn" strings.
>
> That ends up being less than 1KB of static text, which isn't bad at all.
>
> This could potentially speed up e2p_feature2string(), since it could
> index directly into three different arrays instead of having to scan all
> of them to find the right f_compat type first.  I'm not sure if it really
> makes a noticeable difference in the end though.

I can do either, if you insist.  What would you prefer:

a) Statically defining the various FEATURE_foo strings, as you describe.
b) Duplicating the implementation of the two functions so that
e2p_feature2string() can retain its prior behaviour of returning a
string constant in the common cases.

Or alternatively, I suppose:

c) Use a thread-local internal buffer to return the results, so it's
at least thread-safe (though still not really safe, since multiple
calls on one line - e.g. as printf arguments - could still evaluate to
incorrect results).
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html