On 03/01/2018 04:38 PM, Andrew Morton wrote:
> On Thu, 1 Mar 2018 12:43:38 -0500 Waiman Long <longman@xxxxxxxxxx> 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
