Date: Sun, 17 Jul 2011 21:22:38 -0400 From: Ryan Stone <rysto32@gmail.com> To: Bruce Evans <brde@optusnet.com.au> 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 Message-ID: <CAFMmRNxcvLwh=pp5Wu_qmHWdQ78xUMmJtkpYymgxv6kHsFBgxQ@mail.gmail.com> In-Reply-To: <20110718080428.N3939@besplex.bde.org> References: <201107172108.p6HL8Gkd090278@svn.freebsd.org> <20110718080428.N3939@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, Jul 17, 2011 at 7:09 PM, Bruce Evans <brde@optusnet.com.au> 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.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAFMmRNxcvLwh=pp5Wu_qmHWdQ78xUMmJtkpYymgxv6kHsFBgxQ>