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 13:38:07 -0800
From:   Andrew Morton <akpm@...ux-foundation.org>
To:     Waiman Long <longman@...hat.com>
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 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.