Re: [PATCH v3 3/6] sysctl: Add flags to support min/max range clamping

From: Waiman Long
Date: Thu Mar 01 2018 - 16:54:27 EST


On 03/01/2018 04:31 PM, Andrew Morton wrote:
> On Thu, 1 Mar 2018 12:43:37 -0500 Waiman Long <longman@xxxxxxxxxx> wrote:
>
>> When minimum/maximum values are specified for a sysctl parameter in
>> the ctl_table structure with proc_dointvec_minmax() handler, update
>> to that parameter will fail with error if the given value is outside
>> of the required range.
>>
>> There are use cases where it may be better to clamp the value of
>> the sysctl parameter to the given range without failing the update,
>> especially if the users are not aware of the actual range limits.
>> Reading the value back after the update will now be a good practice
>> to see if the provided value exceeds the range limits.
>>
>> To provide this less restrictive form of range checking, a new flags
>> field is added to the ctl_table structure. The new field is a 16-bit
>> value that just fits into the hole left by the 16-bit umode_t field
>> without increasing the size of the structure.
>>
>> When the CTL_FLAGS_CLAMP_RANGE flag is set in the ctl_table entry,
>> any update from the userspace will be clamped to the given range
>> without error.
>>
>> ...
>>
>> --- a/include/linux/sysctl.h
>> +++ b/include/linux/sysctl.h
>> @@ -116,6 +116,7 @@ struct ctl_table
>> void *data;
>> int maxlen;
>> umode_t mode;
>> + uint16_t flags;
> It would be nice to make this have type `enum ctl_table_flags', but I
> guess there's then no reliable way of forcing it to be 16-bit.
> I guess this is the best we can do...
>
> --- a/include/linux/sysctl.h~sysctl-add-flags-to-support-min-max-range-clamping-fix
> +++ a/include/linux/sysctl.h
> @@ -116,7 +116,7 @@ struct ctl_table
> void *data;
> int maxlen;
> umode_t mode;
> - uint16_t flags;
> + uint16_t flags; /* enum ctl_table_flags */
> struct ctl_table *child; /* Deprecated */
> proc_handler *proc_handler; /* Callback for text formatting */
> struct ctl_table_poll *poll;
> @@ -125,7 +125,7 @@ struct ctl_table
> } __randomize_layout;
>
> /**
> - * enum ctl_table_flags - flags for the ctl table
> + * enum ctl_table_flags - flags for the ctl table (struct ctl_table.flags)
> *
> * @CTL_FLAGS_CLAMP_RANGE: Set to indicate that the entry should be
> * flexibly clamped to min/max range in case the user provided

The flags field is actually a bit mask of various bits defined in the
enum type. So I will not change the type of flags to enum
ctl_table_flags as it is not strictly an enum type. However, I can
certainly add the suggested comment change in the next version of the patch.

Cheers,
Longman