lkml.org 
[lkml]   [2018]   [Mar]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v3 3/6] sysctl: Add flags to support min/max range clamping
    From
    Date
    On 03/08/2018 12:51 PM, Luis R. Rodriguez wrote:
    > On Thu, Mar 01, 2018 at 01:31:17PM -0800, Andrew Morton wrote:
    >> On Thu, 1 Mar 2018 12:43:37 -0500 Waiman Long <longman@redhat.com> 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...
    >>

    Actually, I can make the flags just an unsigned integer. I chose
    uint16_t for backporting purpose. For upstream code, it is not a concern
    at all. That will allow more bits for expansion in the future.

    > We can add this to the enum:
    >
    > enum ctl_table_flags {
    > CTL_FLAGS_CLAMP_RANGE = BIT(0),
    > + __CTL_FLAGS_CLAMP_MAX = BIT(16),
    > };
    >
    >
    > Then also:
    >
    > #define CTL_TABLE_FLAGS_ALL ((BIT(__CTL_FLAGS_CLAMP_MAX + 1))-1)
    >
    > at the end of the definition, then a helper which can be used during
    > parsing:
    >
    > static int check_ctl_table_flags(u16 flags)
    > {
    > if (flags & ~(CTL_TABLE_FLAGS_ALL))
    > return -ERANGE;
    > return 0;
    > }

    I don't think that helper function works unless the parameter is changed
    to int instead of u16. I will just change the flags to uint and forget
    about checking for out-of-range bit.

    Cheers,
    Longman

    \
     
     \ /
      Last update: 2018-03-08 20:30    [W:3.057 / U:0.040 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site