lkml.org 
[lkml]   [2015]   [Jun]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: sysctl_writes_strict documentation + an oddity?
Hi Kees,

On 06/16/2015 06:32 PM, Kees Cook wrote:
> On Tue, Jun 16, 2015 at 3:03 AM, Michael Kerrisk (man-pages)
> <mtk.manpages@gmail.com> wrote:
>> On 06/04/2015 09:36 PM, Kees Cook wrote:
>>> On Sat, May 9, 2015 at 1:54 AM, Michael Kerrisk (man-pages)
>>> <mtk.manpages@gmail.com> wrote:
>>>> ===== 2) Behavior puzzle (a) =====
>>>>
>>>> The last sentence quoted from the man page was based on your sentence
>>>>
>>>> Writes to numeric sysctl entries must always be at file position 0
>>>> and the value must be fully contained in the buffer sent in the write
>>>> syscall.
>>>>
>>>> So, I had interpreted /proc/sys/kernel/sysctl_writes_strict==1 to
>>>> mean that if one writes into a numeric /proc/sys file at an offset
>>>> other than zero, the write() will fail with some kind of error.
>>>
>>> Reporting back an error wasn't something I'd tested before. Looking at
>>> the code again now, it should be possible make this change.
>>> Regardless, in the case of the numeric value error condition, it's the
>>> same as the "past the end" string error condition: "Anything written
>>> beyond the maximum length of the value buffer will be ignored." i.e.
>>> anything other than file offset 0 is considered "past the end of the
>>> buffer" for a numeric value and is ignored.
>>>
>>>> But this seems not to be the case. Instead, the write() succeeds,
>>>> but the file is left unmodified. That's surprising, I find. So, I'm
>>>> wondering whether the implementation deviates from your intention.
>>>>
>>>> There's a test program below, which takes arguments as follows
>>>>
>>>> ./a.out pathname offset string
>>>
>>> I have tests in tools/testing/selftests/sysctl for checking the
>>> various behaviors too. They don't actually examine any error
>>> conditions from the sysctl writing itself. It should be simple to make
>>> sysctl_writes_strict failures return an error, though.
>>
>> So, what do you think: is it *desirable* to make sysctl_writes_strict
>> failures return an error?
>
> I think it would be desirable, yes. I want to improve the tests to add
> error checking first, so I can make sure the change doesn't introduce
> anything unexpected. The fix is simple, but since the code is a little
> twisty, I want to be careful.

Okay.

>>>> ===== 2) Behavior puzzle (b) =====
>>>>
>>>> In commit f88083005ab319abba5d0b2e4e997558245493c8, there is this note:
>>>>
>>>> This adds the sysctl kernel.sysctl_writes_strict to control the write
>>>> behavior. The default (0) reports when VFS position is non-0 on a
>>>> write, but retains legacy behavior, -1 disables the warning, and 1
>>>> enables the position-respecting behavior.
>>>>
>>>> The long-term plan here is to wait for userspace to be fixed in response
>>>> to the new warning and to then switch the default kernel behavior to the
>>>> new position-respecting behavior.
>>>>
>>>> (That last para was added to the commit message by AKPM, I see.)
>>>>
>>>> But, I wonder here whether /proc/sys/kernel/sysctl_writes_strict==0
>>>> is going to help with the long-term plan. The problem is that in
>>>> warn_sysctl_write(), pr_warn_once() is used. This means that only
>>>> the first offending user-space application that writes to *any*
>>>> /proc/sys file will generate the printk warning. If that application
>>>> isn't fixed, then none of the other "broken" applications will be
>>>> discovered. It therefore seems possible that it could be a very long
>>>> time before we could "switch the default kernel behavior to the
>>>> new position-respecting behavior".
>>>>
>>>> Looking over old mails
>>>> (http://thread.gmane.org/gmane.linux.kernel/1695177/focus=23240),
>>>> I see that you're aware of the problem, but it seems to me that
>>>> the switch to pr_warn_once() (for fear of spamming the log) likely
>>>> dooms the long-term plan to failure. Your thoughts?
>>>
>>> In actual regular use, the situation that triggers the warning should
>>> be vanishingly rare, but the condition can be trivially met by someone
>>> intending to hit it for the purposes of filling log files. As such, it
>>> makes sense to me to use _once to avoid spamming, but still catch a
>>> rare usage under normal conditions.
>>
>> So, I'm not clear whether you think I'm wrong or not ;-).
>> Do you disagree with my point that this approach may doom
>> the long-term project to failure? (That was my main point.)
>
> Sorry! No, I don't think using pr_warn_once() will doom the
> transition. I think that if we see the warning, we need to investigate
> what's using sysctl that way. If we never see it, then we can switch
> the default. (Using _once protects against log spamming.) I would
> basically expect to never see this warning, but akpm wanted to be very
> cautious, which I can't argue with. :)

Okay.

The man-pages text above will go out with the next release. Thanks
for your help!

Cheers,

Michael

--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/


\
 
 \ /
  Last update: 2015-06-17 11:21    [W:0.047 / U:0.056 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site