Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id ; Thu, 5 Apr 2001 23:34:38 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id ; Thu, 5 Apr 2001 23:34:28 -0400 Received: from www.cscw.msu.edu ([35.8.233.178]:29714 "EHLO cscw.msu.edu") by vger.kernel.org with ESMTP id ; Thu, 5 Apr 2001 23:34:21 -0400 To: linux-kernel@vger.kernel.org Subject: Re: [CHECKER] 3 kmalloc underallocation bugs In-Reply-To: <200104052245.PAA29663@csl.Stanford.EDU> Reply-To: pfaffben@msu.edu MIME-Version: 1.0 (generated by SEMI 1.14.3 - "Ushinoya") Content-Type: text/plain; charset=US-ASCII From: Ben Pfaff Date: 05 Apr 2001 23:33:36 -0400 In-Reply-To: Dawson Engler's message of "Thu, 5 Apr 2001 15:45:47 -0700 (PDT)" Message-ID: <87wv8yg0rz.fsf@pfaffben.user.msu.edu> Lines: 66 User-Agent: Chaos/1.13.1 SEMI/1.14.3 (Ushinoya) FLIM/1.14.2 (Yagi-Nishiguchi) APEL/10.3 Emacs/20.7 (i386-debian-linux-gnu) MULE/4.0 (HANANOEN) Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Dawson Engler writes: > enclosed are three bugs found in the 2.4.1 kernel by an extension > that checks that kmalloc calls allocate enough memory. It examines all > callsites of the form: > p = [kv]malloc(nbytes); > and issues an error if > sizeof *p < nbytes [...] > struct midi_hdr *midihdr; > > Error ---> > if ((midihdr = (struct midi_hdr *) kmalloc(sizeof(struct midi_hdr *), GF > P_KERNEL)) == NULL) { This sort of thing is why the comp.lang.c approved way to call malloc() is foo *x = malloc (sizeof *x); No cast is required and the sizeof usage resembles the declaration. The following is what I say on comp.lang.c when someone does it another way. AFAICS the recommendations apply equally to [kv]malloc(). ---------------------------------------------------------------------- When calling malloc(), I recommend using the sizeof operator on the object you are allocating, not on the type. For instance, *don't* write this: int *x = malloc (sizeof (int) * 128); /* Don't do this! */ Instead, write it this way: int *x = malloc (sizeof *x * 128); There's a few reasons to do it this way: * If you ever change the type that `x' points to, it's not necessary to change the malloc() call as well. This is more of a problem in a large program, but it's still convenient in a small one. * Taking the size of an object makes your sizeof call more similar to your declaration, which makes writing the statement less error-prone. For instance, above, the declaration syntax is `*x' and the sizeof operation is also written `*x'. This provides a visual clue that the malloc() call is correct. I don't recommend casting the return value of malloc(): * The cast is not required in ANSI C. * Casting its return value can mask a failure to #include , which leads to undefined behavior. * If you cast to the wrong type by accident, odd failures can result. ---------------------------------------------------------------------- -- Ben Pfaff MSU Student - Debian GNU/Linux Maintainer - GNU Developer Personal webpage: http://www.msu.edu/user/pfaffben - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/