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

[Luis R. Rodriguez]

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@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...
> 

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;
}

Waiman please evaluate and add.

  Luis