ABI change for setitimer(2) [in feature-removal-schedule.txt]

ABI change for setitimer(2) [in feature-removal-schedule.txt]

[Michael Kerrisk]

[Was: Re: [GIT pull] timer fixes for 3.4]

[CC+=Trevor Woerner, after an independent mail led me to find this thread.]
[CC+=linux-api@, which really should have been CCed on the original patch]

Linus,

On Fri, Apr 13, 2012 at 12:19 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Thu, Apr 12, 2012 at 3:06 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
>>
>> The itimer removal one is not strictly a fix, but I really wanted to
>> avoid a rebase of the urgent ones.
>
> Btw, I think that setitimer NULL pointer removal commit is crap.
>
> If people actually do rely on the NULL pointer thing, we're not
> "scheduling it for removal". And we most *definitely* aren't
> scheduling it for removal for some short timeframe like 3.6.
>
> That's not how ABI's work. If it has become something people rely on,
> it now *is* part of the ABI, and no amount of "violates the spec"
> matters what-so-ever.
>
> "The spec" is paper - and worthless. What people actually *do* is all
> that matters.

So, to restore some context some months after the last mail in this
thread, commit aa2bf9bc6414b6972b9e51903c1ce7b1f057aee2 added this to
Documentation/feature-removal-schedule.txt:

[[
What:   setitimer accepts user NULL pointer (value)
When:   3.6
Why:    setitimer is not returning -EFAULT if user pointer is NULL. This
        violates the spec.
]]

And the commit message said:

    setitimer() should return -EFAULT if called with an invalid pointer
    for value. The current code excludes a NULL pointer from this rule and
    silently uses it to stop the timer. This violates the spec.

The last sentence is rather questionable. POSIX is actually rather
silent on this point, since it doesn't say anything explicit about the
new_value==NULL case. The Linux man page is also rather silent about
it. (By the way, the current behavior seems to have been present on
Linux since 1.0.) So, I'm not sure what spec is being referred to (and
thus, what the rationale is for the change).

In any case, if the behavior here should change, the proposed change
is the wrong change. The right thing to do would be to make Linux
consistent with other systems. At least Solaris, FreeBSD, and NetBSD
(which are all the systems that I have for testing) all do the
following (though none of them seem to document it):

    If the 'new_value' of setitimer() is NULL, then (assuming
    that 'old_value' is not NULL) the call is equivalent to
    getitimer(), simply return the current value of the timer
    in 'old_value'

If Linux is to change, then it should at least be made consistent with
those other systems. There is one question if that path is taken: what
to do in the case where both 'old_value" and 'new_value' are NULL?
Solaris treats that case as a no-op; FreeBSD gives EFAULT. The FreeBSD
behavior seems a little more sensible, IMO.

Would you entertain a patch to implement FreeBSD semantics for
setitimer() where 'new_value' is NULL?

Later in this thread you said:

> I think the whole "let's deprecate this six months into the future" is
> unnecessary. Yes, it may well be worth doing for something with bigger
> consequences, but I think that for something like this, it's just
> overthinking the issue.

When it comes to ABIs, I think there *is* value in a lead time on the
change. This particular example is a good example of why. Making ABI
changes like this should be done with a bit or research and review,
rather than bluntly making the change and seeing if anybody squawks
because something breaks. If this change _had_ been instantly executed
in 3.4, then we'd have missed a chance to bring greater consistency
across systems (here, I assume that having added the EFAULT behavior,
there'd probably be resistance to changing the ABI a *second* time in
order to achieve consistency).

Thanks,

Michael


-- 
Michael Kerrisk Linux man-pages maintainer;
http://www.kernel.org/doc/man-pages/
Author of "The Linux Programming Interface", http://blog.man7.org/

Re: ABI change for setitimer(2) [in feature-removal-schedule.txt]

[Linus Torvalds]

On Sat, Sep 29, 2012 at 11:30 PM, Michael Kerrisk
<mtk.manpages@gmail.com> wrote:
>
>> I think the whole "let's deprecate this six months into the future" is
>> unnecessary. Yes, it may well be worth doing for something with bigger
>> consequences, but I think that for something like this, it's just
>> overthinking the issue.
>
> When it comes to ABIs, I think there *is* value in a lead time on the
> change. This particular example is a good example of why.

No. This whole example is a good example of the fact that YOU SHOULD
NOT MAKE ABI CHANGES.

I don't understand why this seems to be so hard for people to understand.

There are exactly *zero* reasons to change the ABI for its own sake,
and this whole thread is a wonderful example of how F*CKING STUPID it
was to even consider it.

There are real and valid reasons to change the ABI, but for every
single one of them, there is some external issue:

 - security. We've had cases where we had an ABI that simply exposed
too much information.

 - implementation issues. Sometimes, we've done something really
really badly, and some subtle ABI issue may simply not work. This is
basically never about normal system calls used by normal applications,
though - it's about things like the whole iptables flaps etc.

 - actual real applications breaking. We've had cases where we simply
did things wrong, and portable applications broke. Then we can *try*
to fix it, and see if something else breaks from that.

And quite frankly, for all but the security case, even then we're
often better off at least having a compatibilty layer for the old
cases, even if it was bad and wrong (example: the very original linux
'select()' timeout behavior, where Linux did the documented thing, but
nobody else did. Or the various versions of 'stat()' we've had. Or the
inotify/dnotify/fsnotify things).

Occasionally some compat model may not be worth it (if the interface
is too specialized and there really is just one or two system apps
that use it), but that's very very rare to the point where it
shouldn't even be considered an issue.

Quite frankly, our most common ABI change is that we don't even
realize that something changed. And then people may or may not notice
it. And we've had cases where the same system call returned
*different* things for different subsystems, and we tried to make it
at least internally consistent.

But the "premeditated ABI change just for the reason of an ABI
change"? It's bullshit. And it's bullshit whether it shows up in
feature-removal or not.

(The whole feature-removal file is BS, for that matter, but that's a
different issue).

SO STOP DOING ABI CHANGES. WE DON'T DO THEM.

The absolute worst thing a kernel can do is "change the user-level
interfaces". It has to be done occasionally (see above), and sometimes
we do it by mistake, but anybody who does it on purpose "just because"
should not be involved in kernel development (or library development
for that matter).

                Linus

Re: ABI change for setitimer(2) [in feature-removal-schedule.txt]

[Michael Kerrisk]

On Sun, Sep 30, 2012 at 7:51 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Sat, Sep 29, 2012 at 11:30 PM, Michael Kerrisk
> <mtk.manpages@gmail.com> wrote:
>>
>>> I think the whole "let's deprecate this six months into the future" is
>>> unnecessary. Yes, it may well be worth doing for something with bigger
>>> consequences, but I think that for something like this, it's just
>>> overthinking the issue.
>>
>> When it comes to ABIs, I think there *is* value in a lead time on the
>> change. This particular example is a good example of why.
>
> No. This whole example is a good example of the fact that YOU SHOULD
> NOT MAKE ABI CHANGES.
>
> I don't understand why this seems to be so hard for people to understand.
>
> There are exactly *zero* reasons to change the ABI for its own sake,
> and this whole thread is a wonderful example of how F*CKING STUPID it
> was to even consider it.

[...]

> Quite frankly, our most common ABI change is that we don't even
> realize that something changed. 

(Yes.)

> And then people may or may not notice
> it. And we've had cases where the same system call returned
> *different* things for different subsystems, and we tried to make it
> at least internally consistent.
>
> But the "premeditated ABI change just for the reason of an ABI
> change"? It's bullshit. And it's bullshit whether it shows up in
> feature-removal or not.
>
> (The whole feature-removal file is BS, for that matter, but that's a
> different issue).
>
> SO STOP DOING ABI CHANGES. WE DON'T DO THEM.
>
> The absolute worst thing a kernel can do is "change the user-level
> interfaces". It has to be done occasionally (see above), and sometimes
> we do it by mistake, but anybody who does it on purpose "just because"
> should not be involved in kernel development (or library development
> for that matter).

Agreed. As I pointed out, the reason for this proposed change is 
dubious at best. There is no "spec" on this point. And though I 
didn't mention it (since it seemed obvious), no one has mentioned 
any user-space hardship because of current behavior.

Given the choice of (1) no change, (2) making the proposed dubious 
change, or (3) making a change to make Linux consistent with other 
systems, (1) is obviously the best in this case. The only thing that 
surprised me was that you and Thomas merged this proposal into 
feature-removal-schedule.txt, which seemed to indicate an agreed 
intent to change the ABI (i.e., discarding option (1)), and
if so, I wanted to point out that proposed direction was wrong.

Patch follows.

Thanks,

Michael

[patch] Remove proposed ABI change for setitimer(2)

[Michael Kerrisk]

Commit aa2bf9bc6414b6972b9e51903c1ce7b1f057aee2 scheduled an ABI
change for setitimer() whose rationale is dubious. The standards
are largely silent on the point referred to in that commit's 
changelog. In addition, current behavior is causing no reported
hardship for user-space. So, there are no benefits to the change,
and making it risks breaking userspace applications that rely
on the (so far undocumented) Linux behavior. Finally, if one even 
contemplated making a change to the ABI on this point, the sensible 
change would be to make Linux behave as Solaris and the BSDs where:
setitimer(..., NULL, &ovalue) == getitimer(..., &ovalue)

The sensible thing is no change at all. This patch removes the 
scheduled change from feature-removal-schedule.txt.

Signed-off-by: Michael Kerrisk <mtk.man-pages@gmail.com>

---
diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
index f4d8c71..cdf4ded 100644
--- a/Documentation/feature-removal-schedule.txt
+++ b/Documentation/feature-removal-schedule.txt
@@ -528,14 +528,6 @@ Who:	Samuel Ortiz <sameo@linux.intel.com>
 
 ----------------------------
 
-What:	setitimer accepts user NULL pointer (value)
-When:	3.6
-Why:	setitimer is not returning -EFAULT if user pointer is NULL. This
-	violates the spec.
-Who:	Sasikantha Babu <sasikanth.v19@gmail.com>
-
-----------------------------
-
 What:	remove bogus DV presets V4L2_DV_1080I29_97, V4L2_DV_1080I30 and
 	V4L2_DV_1080I25
 When:	3.6