From owner-freebsd-hackers@FreeBSD.ORG Fri Sep 17 09:37:25 2004 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 3082916A4CE for ; Fri, 17 Sep 2004 09:37:25 +0000 (GMT) Received: from kane.otenet.gr (kane.otenet.gr [195.170.0.27]) by mx1.FreeBSD.org (Postfix) with ESMTP id 75F8E43D5E for ; Fri, 17 Sep 2004 09:37:24 +0000 (GMT) (envelope-from keramida@freebsd.org) Received: from orion.daedalusnetworks.priv (host5.bedc.ondsl.gr [62.103.39.229])i8H9bK6X023120; Fri, 17 Sep 2004 12:37:21 +0300 Received: from orion.daedalusnetworks.priv (orion [127.0.0.1]) i8H9bDUT095303; Fri, 17 Sep 2004 12:37:13 +0300 (EEST) (envelope-from keramida@freebsd.org) Received: (from keramida@localhost)i8H9bCNX095302; Fri, 17 Sep 2004 12:37:12 +0300 (EEST) (envelope-from keramida@freebsd.org) Date: Fri, 17 Sep 2004 12:37:12 +0300 From: Giorgos Keramidas To: gerarra@tin.it Message-ID: <20040917093712.GB94990@orion.daedalusnetworks.priv> References: <4146316C00007833@ims3a.cp.tin.it> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii; x-action=pgp-signed Content-Disposition: inline In-Reply-To: <4146316C00007833@ims3a.cp.tin.it> cc: freebsd-hackers@freebsd.org Subject: Re: FreeBSD Kernel buffer overflow X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 Sep 2004 09:37:25 -0000 -----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-----