linux-kernel - Re: [PATCH v2 1/3] trace-cmd: Make read_proc() to return int status via OUT arg

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]

Message-ID: <20180116143432.5b7d9219@gandalf.local.home>
Date:   Tue, 16 Jan 2018 14:34:32 -0500
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Vladislav Valtchev <vladislav.valtchev@...il.com>
Cc:     y.karadz@...il.com, linux-trace-devel@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/3] trace-cmd: Make read_proc() to return int status
 via OUT arg

On Tue, 16 Jan 2018 20:49:22 +0200
Vladislav Valtchev <vladislav.valtchev@...il.com> wrote:


> Sure, using two checks with 'int' is less efficient then using the 'unsigned trick',
> but my point is that such a function (at interface level) should accept exactly
> the same type 'returned' (via OUT param) by read_proc(). It should be symmetric,
> as if instead of 'int/unsigned' we used an opaque type 'value_t' for which we cannot
> make assumptions. Clearly, the implementation may in practice accept a subset of the values
> allowed by the parameter type. 
> 
> What about accepting 'int' but doing the check this way:
> 
> 	if ((unsigned)new_status > 9) {
> 		warning(...);
> 		return;
> 	}
> 
> This way, we'll keep the interface symmetric (with read_proc()) but, at the same time,
> we use a more efficient check.

The thing is, since we only accept new_status to be 0-9, it can never
be negative. Thus we should make it unsigned. Yes read_proc() is
signed, but that's because it can be negative.

We are keeping the variables the same as the values they represent,
nothing more. One variable shouldn't affect what the other variable is,
as the ranges are indeed different.

-- Steve