From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1162594AbeCAWWe (ORCPT ); Thu, 1 Mar 2018 17:22:34 -0500 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:51212 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1162548AbeCAWWc (ORCPT ); Thu, 1 Mar 2018 17:22:32 -0500 Subject: Re: [PATCH v3 4/6] sysctl: Warn when a clamped sysctl parameter is set out of range To: Andrew Morton Cc: "Luis R. Rodriguez" , Kees Cook , linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, Al Viro , Matthew Wilcox References: <1519926220-7453-1-git-send-email-longman@redhat.com> <1519926220-7453-5-git-send-email-longman@redhat.com> <20180301133807.f5beb1bf1d1391f23e95ce63@linux-foundation.org> From: Waiman Long Organization: Red Hat Message-ID: <478592ec-3550-0e81-6242-41190bea6521@redhat.com> Date: Thu, 1 Mar 2018 17:22:31 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0 MIME-Version: 1.0 In-Reply-To: <20180301133807.f5beb1bf1d1391f23e95ce63@linux-foundation.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 03/01/2018 04:38 PM, Andrew Morton wrote: > On Thu, 1 Mar 2018 12:43:38 -0500 Waiman Long 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