linux-kernel - Re: [PATCH 32/32] ver_linux: 'printversion()' function definition

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 for Android: free password hash cracker in your pocket

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

Date:	Sat, 2 Jul 2016 10:29:03 +0300
From:	Alexander Kapshuk <alexander.kapshuk@...il.com>
To:	linux-kernel <linux-kernel@...r.kernel.org>
Cc:	Valdis Kletnieks <Valdis.Kletnieks@...edu>,
	Greg KH <gregkh@...uxfoundation.org>
Subject: Re: [PATCH 32/32] ver_linux: 'printversion()' function definition

On Tue, Jun 28, 2016 at 5:48 PM,  <Valdis.Kletnieks@...edu> wrote:
> On Tue, 28 Jun 2016 17:40:36 +0300, Alexander Kapshuk said:
>
>> Seeing this is a complete rewrite of the script from the shell
>> language into awk, one would not be able to apply the patches
>> submitted incrementally to be able to test each change being
>> introduced separately. In this respect, defining the 'version()' and
>> the 'printversion()' functions ahead of the code that calls them makes
>> no difference.
>
> Good point for this instance.  Note that reviewers in general expect
> that sort of "must still work after every incremental commit" structure
> in the future, though...
>
>
>> If a utility being queried is not available on a given system, the
>> shell that executes the script outputs an error  message along the
>> lines of 'ver_linux: line number where the call is made: error
>> message: name_of_utility Not found or something like that. This is
>> taken care of by the 'if' block in the 'version()' function:
>> if (!/ver_linux/...) {}
>>
>> The second condition that must be met in the 'if' block above takes
>> care of situations where input does not match the regular expression
>> for the version number:
>> if (... && match($0, /[0-9]+([.]?[0-9]+)+/)) {}
>>
>> Only when the 'if' block above evaluates as being true is the 'ver'
>> variable set to the string matched by the regular expression.
>> The 'printversion()' function will not print anything should the value
>> representing the version number, a list of kernel modules, etc, be
>> found empty.
>>
>> If I understood your commentary correctly, the proposed implementation
>> does address the issues you raise. Unless I misread something.
>
> I meant that the distinction should be surfaced to the user - if the
> binary returns a version string that our regexp can't parse, it's probably
> a complete rewrite and we should *tell* the user that we don't know what's
> going on.   An "if result = empty then print '(unable to identify version)'"
> should be sufficient....
>

I do not mind reworking the script to make it report errors to the
user, if that is the behaviour expected by the community. The proposed
implementation follows the behaviour of the previous iterations of the
script based on its development history, which is to be silent about
those kinds of errors.

I have just been waiting for more feedback from the community before
making any further modifications to the proposed implementation.

Thanks.