linux-kernel - Re: [PATCH v3 4/6] sysctl: Warn when a clamped sysctl parameter is set out of range

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:   Thu, 1 Mar 2018 17:22:31 -0500
From:   Waiman Long <longman@...hat.com>
To:     Andrew Morton <akpm@...ux-foundation.org>
Cc:     "Luis R. Rodriguez" <mcgrof@...nel.org>,
        Kees Cook <keescook@...omium.org>,
        linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        Al Viro <viro@...iv.linux.org.uk>,
        Matthew Wilcox <willy@...radead.org>
Subject: Re: [PATCH v3 4/6] sysctl: Warn when a clamped sysctl parameter is
 set out of range

On 03/01/2018 04:38 PM, Andrew Morton wrote:
> On Thu,  1 Mar 2018 12:43:38 -0500 Waiman Long <longman@...hat.com> wrote:
>
>> Even with clamped sysctl parameters, it is still not that straight
>> forward to figure out the exact range of those parameters. One may
>> try to write extreme parameter values to see if they get clamped.
>> To make it easier, a warning with the expected range will now be
>> printed in the kernel ring buffer when a clamped sysctl parameter
>> receives an out of range value.
>>
>> ...
>>
>> +		if (clamped && param->name &&
>> +		   !(*param->flags & CTL_FLAGS_OOR_WARNED)) {
>> +			proc_ctl_warn(d, param->name,
>> +				param->min ? *param->min : -INT_MAX,
>> +				param->max ? *param->max :  INT_MAX, val);
>> +			*param->flags |= CTL_FLAGS_OOR_WARNED;
>> +		}
> The handling of ctl_table.flags looks racy on SMP or preemptible. 
> That's not at all a serious problem in this usage, but such handling of
> ctl_table.flags may be a problem in the future.  Which means that if
> some future user of this field *is* sensitive to races then people are
> going to have to come back to this code and add the needed locking.
>
> So we should at least think about what that locking is to be, and
> document it in some fashion.  Do we already hold an appropriate lock at
> this time?  If so, what is it?
>
> If some such future user of ctl_table.flags has to add a new lock to
> the ctl_table for this purpose then we just eliminated your use-16-bit
> space saving trick and we may as well use a ulong and operate on it
> with bitops.

We don't actually need locking if it is only the flags that we are
worrying about. Doing some kind of atomic bit setting should be enough.
I should probably add some comment to elaborate a bit more on this.
Thanks for reminding me about this forward looking concern.

Cheers,
Longman