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>
index | next in thread | previous in thread | raw e-mail
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
>> if (isalpha(*cp))
>> command->args[command->n_args].argtype = *cp++;
>> end = NULL;
>> - command->args[command->n_args].arg_val = strtol(cp, &end, 0);
>> + command->args[command->n_args].arg_val = strtoul(cp, &end, 0);
>> if (cp == end || (!isspace(*end) && *end != '\0')) {
>> char ch;
>> end = cp;
>>
>
> Also, arg_val is never used as a u_long. We risk truncation when it is
> blindly assigned to variables whose type is never u_long. It 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. This style bug
> was imported from Mach and never fixed). Since uint is unsigned and ints
> have 32 bits, things now work up to UINT32_MAX but no further. arg_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.
help
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAFMmRNxcvLwh=pp5Wu_qmHWdQ78xUMmJtkpYymgxv6kHsFBgxQ>