Date: Fri, 1 Apr 2005 08:18:33 -0600 From: "Jacques A. Vidrine" <nectar@FreeBSD.org> To: Roberto <roberto.trovo@redix.it> Cc: Colin Percival <cperciva@freebsd.org> Subject: Re: FreeBSD Security Advisory FreeBSD-SA-05:01.telnet Message-ID: <20050401141833.GF4455@lum.celabo.org> In-Reply-To: <1068.192.168.0.150.1112340588.squirrel@mail.redix.it:443> References: <1112296855.8421.64.camel@localhost> <424C7B88.9030605@freebsd.org> <1068.192.168.0.150.1112340588.squirrel@mail.redix.it:443>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Apr 01, 2005 at 09:29:48AM +0200, Roberto wrote:
> Actually I've not read the code,
Then why are you posting your opinion about it? (^_^)
I guess I'm responding to your post only to prevent others from worrying
about a non-existent ``problem''.
> but from these email it seems to me that
> someone could be confused by this code (at least Steve and I); for example
> refer to the address "&slc_reply[128];" when slc_reply[127] is the last
> element.
There is no reference to ``&slc_reply[128]''. There *is* a pointer
initialized to the equivalent expression
``&slc_reply[sizeof(slc_reply)]'', which is the usual way to designate
the end of a sequence. For example,
char buf[...];
const char *eom = &buf[sizeof(buf)];
while (p < eom) /* `*p' is valid */;
size_t n = eom - p; /* There are `n' bytes left */
If we used a pointer to the last element (instead of one beyond the last
element), we'd need to adjust many expressions by 1, which is
error-prone and ugly.
> I do not want to be offensive in any way, what I want to say is that this
> code is clear to you (and the person who wrote it) but the next programmer
> that will reuse the code (because this is a open source) could make a
> mistake.
>
> I think many bugs can derive from code not easy to understand.
>
> This is only my opinion.
I find the tests fairly idiomatic and I find it easy to see their
correctness. I doubt I'm alone. The suggested fix was reviewed by a
number of coders from several open source operating system projects and
caused no confusion. The form was chosen to clearly show how many bytes
were expected to be written at that point. IMHO, using alternative
forms invites off-by-one errors.
if (&slc_replyp[6+2] > slc_reply_eom)
return;
/* past this point, we can write 6+2 bytes, slc_replyp[0]
* through slc_replyp[7].
*/
Cheers,
--
Jacques A Vidrine / NTT/Verio
nectar@celabo.org / jvidrine@verio.net / nectar@FreeBSD.org
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20050401141833.GF4455>