Skip to contentSkip to conversation

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

PATCH New Michael Kerrisk 2 participants 4 messages Sep 30, 2012 – Oct 1, 2012

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

[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,

Quoted 11 lines · Linus Torvalds
Quoted 3 lines · Thomas Gleixner

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.

Quoted 6 lines

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?

Quoted 4 lines · Later in this thread you

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

Signature · 3 lines
Michael Kerrisk Linux man-pages maintainer;
http://www.kernel.org/doc/man-pages/
Author of "The Linux Programming Interface", http://blog.man7.org/
#1 lore raw
Quoted 8 lines · Michael Kerrisk
Quoted 5 lines

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

#2 lore raw
Quoted 19 lines · Linus Torvalds
Quoted 8 lines · Michael Kerrisk
Quoted 5 lines

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.

[...]

Quoted 2 lines

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

(Yes.)

Quoted 12 lines

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

Quoted 7 lines

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

#3 lore raw

PATCH 1/1 Remove proposed ABI change for setitimer(2)

In reply to Linus Torvalds

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.

---

Documentation/feature-removal-schedule.txt +0 -8
index f4d8c71..cdf4ded 100644
@@ -528,14 +528,6 @@ Who:	Samuel Ortiz <[email protected]>
 
 ----------------------------
 
-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 <[email protected]>
-
-----------------------------
-
 What:	remove bogus DV presets V4L2_DV_1080I29_97, V4L2_DV_1080I30 and
 	V4L2_DV_1080I25
 When:	3.6