Re: [PATCH 4/4] device/chario.c (char_write): avoid segmentation fault

[Marin Ramesa]

On 14.12.2013 12:45:32, Richard Braun wrote:
> On Fri, Dec 13, 2013 at 09:06:55PM +0100, Marin Ramesa wrote:
> > Check if the source of the data in the source structure
> > memory is a target structure data. Do this by comparing the 
> > length of the char values starting with the source address until 
> > null-termination to the size of the target structure. Start by 
> > initializing the counter to the size of the first member in the 
> > target structure. Then char values in memory are counted until 
> > null-termination, and finally the counted result is compared to the 
> > size of the target structure. If the values don't match, return 
> > from the function.
> 
> I really don't see a problem in that code, you'll have to describe it
> better. 

So a pointer to io_data is cast to a pointer to a vm_map_copy structure 
and then passed to vm_map_copyout() which uses the vm_map_copy 
structure members as if the contents of the memory at the source 
address were vm_map_copy structure objects, not objects of io_data 
which is of unknown size and origin to the function. This can lead to a 
segmentation fault, as is shown in that short test program.

My patch makes sure the io_data actually came from a vm_map_copy 
structure.

> On the other hand, your patch relies on knowing the target
> types, although vm_map_copy_t is explicitely described as being
> opaque.

I don't see a problem with it. There is an explicit cast to 
vm_map_copy_t. It's first member type is known.

But maybe I don't understand what it means for a type to be opaque.

> In addition, you basically reimplement strlen inline, 

OK then, I will take a look at strlen.

> and I also don't understand why you assume the data to be null-
> terminated.

Structures are always null-terminated in memory. So if data came from a 
structure (like vm_map_copy) it has to be null-terminated. If not, a 
random '\0' will be found, sizes will not match, and function will 
return.