Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 17 Sep 2004 12:37:12 +0300
From:      Giorgos Keramidas <keramida@freebsd.org>
To:        gerarra@tin.it
Cc:        freebsd-hackers@freebsd.org
Subject:   Re: FreeBSD Kernel buffer overflow
Message-ID:  <20040917093712.GB94990@orion.daedalusnetworks.priv>
In-Reply-To: <4146316C00007833@ims3a.cp.tin.it>
References:  <4146316C00007833@ims3a.cp.tin.it>

next in thread | previous in thread | raw e-mail | index | archive | help
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 2004-09-17 03:37, gerarra@tin.it wrote:
> >If we put your patch in but as a KASSERT then anyone ruinning with
> >debugging turned on
> >(and no-one in their right mind would write a kernel module without
> >turning on debugging, right?)
> >will immediatly find the problem.
>
> What you can't understand is that having a limit about arguments is wrong
> (it's not documented too). Why limiting to 8 and not to 20? or 65? i don't
> understand...

As you have noted in a previous post it's probably more efficient to
have a static limit than a fully dynamic implementation.  I'm sure we
can find a good way to document this and warn the developer who's about
to shoot his feet off about potential problems.

Would you feel that this limitation of syscall() is not really so important
or dangerous if we documented the 8-argument limit, described a good way to
pass more arguments and added a KASSERT in trap.c that is only enabled for
kernels compiled with INVARIANTS turned on?

% Index: lib/libc/sys/syscall.2
% ===================================================================
% RCS file: /home/ncvs/src/lib/libc/sys/syscall.2,v
% retrieving revision 1.11
% diff -u -r1.11 syscall.2
% --- lib/libc/sys/syscall.2      10 Sep 2003 19:24:33 -0000      1.11
% +++ lib/libc/sys/syscall.2      17 Sep 2004 09:35:44 -0000
% @@ -56,14 +56,26 @@
%  interface has the specified
%  .Fa number
%  with the specified arguments.
% +If non-zero, the number of arguments that a system call can have in
% +.Fx
% +should be at most 8.
%  Symbolic constants for system calls can be found in the header file
%  .In sys/syscall.h .
%  The
%  .Fn __syscall
%  form should be used when one or more of the arguments is a
%  64-bit argument to ensure that argument alignment is correct.
% -This system call is useful for testing new system calls that
% +.Pp
% +The
% +.Fn syscall
% +and
% +.Fn __syscall
% +functions are useful for testing new system calls that
%  do not have entries in the C library.
% +If new system calls require more than 8 arguments you can always wrap
% +these arguments in a
% +.Vt struct
% +and pass a pointer to the new system call.
%  .Sh RETURN VALUES
%  The return values are defined by the system call being invoked.
%  In general, a 0 return value indicates success.
% % Index: sys/i386/i386/trap.c
% ===================================================================
% RCS file: /home/ncvs/src/sys/i386/i386/trap.c,v
% retrieving revision 1.269
% diff -u -r1.269 trap.c
% --- sys/i386/i386/trap.c        31 Aug 2004 07:34:53 -0000      1.269
% +++ sys/i386/i386/trap.c        17 Sep 2004 09:21:55 -0000
% @@ -965,6 +965,9 @@
%                 callp = &p->p_sysent->sv_table[code];
%
%         narg = callp->sy_narg & SYF_ARGMASK;
% +#ifdef INVARIANTS
% +       KASSERT(0 <= narg && narg <= 8, ("invalid number of syscall args"));
% +#endif
%
%         /*
%          * copyin and the ktrsyscall()/ktrsysret() code is MP-aware

> In my opinion a patch would be better (and even quicker respect KASSERT).

A KASSERT() wrapped in #ifdef INVARIANTS has zero overhead for normal,
non-debugging kernels.  The developers who are responsible for writing and
testing new system calls should use INVARIANTS anyway, so they'll quickly
catch the mistake.

- - Giorgos

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.6 (FreeBSD)

iD8DBQFBSrBI1g+UGjGGA7YRAk7RAJ9mfyFTYEzZNK5mDel0lqUom+UayACgpwU1
BF+ypfahuqM4ADVIx6HzO9I=
=HLEr
-----END PGP SIGNATURE-----



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040917093712.GB94990>