From owner-svn-src-head@FreeBSD.ORG Mon Jul 18 01:22:40 2011 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 699C8106566B; Mon, 18 Jul 2011 01:22:40 +0000 (UTC) (envelope-from rysto32@gmail.com) Received: from mail-ey0-f176.google.com (mail-ey0-f176.google.com [209.85.215.176]) by mx1.freebsd.org (Postfix) with ESMTP id A3D388FC0C; Mon, 18 Jul 2011 01:22:39 +0000 (UTC) Received: by eya28 with SMTP id 28so2136346eya.21 for ; Sun, 17 Jul 2011 18:22:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; bh=15ExDFnKGWFUbQkpHhKLSVFw+eYS4IqrBGzScYcv/hs=; b=C9ye9RT7E7gxwl/Qyo33smf3pxByytqTtr7WLdxVl8uDQhg+wV7lkKkSez4nn3hGEM pX4gOKrOuL3DkUL0uvyHpda29kKd0AQ7hA9CE7Re6FP7EggXjY+sPG6iuP6l+9+X5noP h7dpU3iAo9cn7mYeI1UnIdGt1QloKpqAXupbc= MIME-Version: 1.0 Received: by 10.213.29.196 with SMTP id r4mr931ebc.52.1310952158380; Sun, 17 Jul 2011 18:22:38 -0700 (PDT) Received: by 10.213.15.70 with HTTP; Sun, 17 Jul 2011 18:22:38 -0700 (PDT) In-Reply-To: <20110718080428.N3939@besplex.bde.org> References: <201107172108.p6HL8Gkd090278@svn.freebsd.org> <20110718080428.N3939@besplex.bde.org> Date: Sun, 17 Jul 2011 21:22:38 -0400 Message-ID: From: Ryan Stone To: Bruce Evans Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, markjdb@gmail.com Subject: Re: svn commit: r224150 - head/sbin/fdisk X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 18 Jul 2011 01:22:40 -0000 On Sun, Jul 17, 2011 at 7:09 PM, Bruce Evans wrote: > There was no need to further break the style. Ack. Should have caught that. Will fix. >> >> @@ -990,7 +990,7 @@ parse_config_line(char *line, CMD *comma >> =A0 =A0 =A0 =A0 =A0 =A0if (isalpha(*cp)) >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0command->args[command->n_args].argtype = =3D *cp++; >> =A0 =A0 =A0 =A0 =A0 =A0end =3D NULL; >> - =A0 =A0 =A0 =A0 =A0 command->args[command->n_args].arg_val =3D strtol(= cp, &end, 0); >> + =A0 =A0 =A0 =A0 =A0 command->args[command->n_args].arg_val =3D strtoul= (cp, &end, 0); >> =A0 =A0 =A0 =A0 =A0 =A0if (cp =3D=3D end || (!isspace(*end) && *end !=3D= '\0')) { >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0char ch; >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0end =3D cp; >> > > Also, arg_val is never used as a u_long. =A0We risk truncation when it is > blindly assigned to variables whose type is never u_long. =A0It is mostly > assigned to variables of type uint ("uint" is an archaic SysV spelling > of the BSD u_int which is a spelling of unsigned int. =A0This style bug > was imported from Mach and never fixed). =A0Since uint is unsigned and in= ts > have 32 bits, things now work up to UINT32_MAX but no further. =A0arg_val > is also blindly assigned to a variable of type int, the partition number, > and to a variable of type u_char (the partition type), but since these > values nonsense unless they are small, there is no problem with overflow > near INT32_MAX or UINT32_MAX. Hm, I was under the impression that fdisk was checking whether the size parameter fit within in 32-bits before putting it in the MBR, but I seem to have been mistaken. In that case, I think that the right fix is to make arg_val a u_int for now, which will still allow slices with more than 2^31 sectors to be specified. Truncation is still an issue but that's not a new bug.