lkml.org 
[lkml]   [2017]   [Nov]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/2] staging: unisys: visorbus: address theoretical int overflows
> --- a/drivers/staging/unisys/visorbus/visorchipset.c
> +++ b/drivers/staging/unisys/visorbus/visorchipset.c
> @@ -581,7 +581,8 @@ static void *parser_name_get(struct parser_context *ctx)
> struct visor_controlvm_parameters_header *phdr;
>
> phdr = &ctx->data;
> - if (phdr->name_offset + phdr->name_length > ctx->param_bytes)
> + if ((unsigned long)phdr->name_offset +
> + (unsigned long)phdr->name_length > ctx->param_bytes)
> return NULL;
> ctx->curr = (char *)&phdr + phdr->name_offset;
> ctx->bytes_remaining = phdr->name_length;


I haven't reviewed this code very thouroughly. This should fix the
issue on 64 bit systems, but it's a no-op on 32 bit systems. Which
might be fine? I would be more comfortable if we just checked for
integer overflow explicitly. There are bunch of ways to do that:

if (phdr->name_offset > ctx->param_bytes ||
phdr->name_length > ctx->param_bytes ||
phdr->name_offset + phdr->name_length > ctx->param_bytes)

Or:

if (phdr->name_offset + phdr->name_length < phdr->name_offset ||
phdr->name_offset + phdr->name_length > ctx->param_bytes)
return NULL;

regards,
dan carpenter


\
 
 \ /
  Last update: 2017-11-28 15:14    [W:0.078 / U:0.064 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site