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

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]

Message-ID: <942e75ec-3a91-e3ca-4cd8-447bf23cef1e@redhat.com>
Date:   Thu, 8 Mar 2018 14:30:04 -0500
From:   Waiman Long <longman@...hat.com>
To:     "Luis R. Rodriguez" <mcgrof@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>
Cc:     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 3/6] sysctl: Add flags to support min/max range
 clamping

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