vmsplice(): the making of a local root exploit [LWN.net]

vmsplice(): the making of a local root exploit

Posted Feb 12, 2008 16:42 UTC (Tue) by utoddl (subscriber, #1232)
In reply to: vmsplice(): the making of a local root exploit by tialaramex
Parent article: vmsplice(): the making of a local root exploit

I basically agree with what you're saying, but I'd like to make just a couple of counter-points.

...beat it into them that useless (e.g. i++; /* increment i */) explanations are worse than none at all.

True, that's useless to you and me, but for the noob programmer who has come through elementary and high school maths, the expression i++ is not immediately obvious. By the second or third C program he should be familiar with it, certainly. But my point is that the "document what's not obvious" standard requires a judgment call where "what's obvious" varies greatly with the experience of the coder.

For example, if you work on code related to SCSI or filesytems or otherwise connected with disks, you're expected to recognise that (bytes >> 9) converts from a byte count to a sector count, since sectors are 512 bytes.

If you say so. I don't happen to work in that domain often, so it isn't obvious to me. However, a well-crafted macro BYTES2SECTORS(bytes) would give me a clue, and the magic numbers and operations on them would live in the definition of the macro, so you've got one place to maintain the conversion and it's clear to us noobs what it's related to. Again, the point isn't about this particular case, but if you can give magic numbers and operations on them a name, you can make the intent more clear (and perhaps avoid a problem, like (bytes<9)). It's already clear to somebody, but will it be clear next week, or to the next programmer who may be less experienced?

(Log in to post comments)

vmsplice(): the making of a local root exploit

Posted Feb 12, 2008 18:14 UTC (Tue) by iabervon (subscriber, #722) [Link]

It is useless to the noob programmer who is looking at a piece of code with hundreds of
thousands of occurrences of the increment operator if they're all commented. It's worse than
useful to that same programmer if some but not all of them are commented (are the commented
ones somehow different?). The novice should have access to a tutorial and reference guide for
the language, and that should explain the notation. Even things which are unique to a
project's style shouldn't be documented in situ at every (or any) place they're used; they
should be documented once in a central location. So the standard should really be "comment
what's unusual about this location, given the context of the rest of the project; also
document what's unusual about this project, given the language it's written in; also have
language reference works available; and certainly don't duplicate any of this information,
since it will just get out of sync."

vmsplice(): the making of a local root exploit

Posted Feb 14, 2008 6:38 UTC (Thu) by jimparis (subscriber, #38647) [Link]

> > For example, if you work on code related to SCSI or filesytems or otherwise connected with
disks, you're expected to recognise that (bytes >> 9) converts from a byte count to a sector
count, since sectors are 512 bytes.

> However, a well-crafted macro BYTES2SECTORS(bytes) would give me a clue, and the magic
numbers and operations on them would live in the definition of the macro, so you've got one
place to maintain the conversion and it's clear to us noobs what it's related to.

But in the context of the kernel, hiding things in a BYTES2SECTORS macro can be downright
dangerous.  What types does it handle for input and output?  Does it sleep or have any locking
requirements?  If I pass an expression, do I have to worry about side effects if it gets
evaluated twice?  How does it behave if bytes is not a multiple of one sector?
Is it talking about the canonical 512-byte defintion of a sector or the actual 2048 byte
sectors used on CD-ROMs?

"bytes >> 9" is absolutely clear.  I see instantly that it handles integer types and matches
the signedness.  It is free from side effects and fast.  I know that the answer is rounded
down to the nearest whole sector.  Sectors are 512 bytes.

I don't see why you would want to hide all of that information from a developer.  Your version
is only simpler at a quick glance to a casual observer, not someone who actually has to deal
with the code and what the statement actually does.