[PATCH] fbsplash: Fix mmap size and offset calculations

[Georges Savoundararadj]

Hi Timo,

On 05/10/2016 11:23 PM, Timo Teras wrote:
> On Tue, 10 May 2016 22:53:49 -0700
> savoundg at gmail.com wrote:
>
>> From: Georges Savoundararadj <savoundg at gmail.com>
>>
>> Before the commit 82c2fad, we were mapping the frame buffer device
>> with the size: yres * line_length.
>> This leads to a segmentation fault if the computed offset (yoffset *
>> line_length + xoffset * bytes_per_pixel) is greater than the size.
>>
>> This commit maps the frame buffer device with the right offset
>> avoiding the need to map to a larger size as done in commit 82c2fad
>> (by using yres_virtual (if non-zero) instead of yres).
> Does this actually work?
Yes, it was working in my environment.
>   mmap requires offset to be aligned by
> PAGE_SIZE to work (it returns error otherwise), so even if it works in
> your environment, I doubt this works with different combinations of
> y-offset, line length and bytes per pixes.
You are right, the offset should be a multiple of the page size.
I was wondering why the offset argument was not used instead of adding 
the offset to the base address G.addr.

We could align the offset to the page size and fix the size accordingly. 
What do you think?

Thanks,

Georges
>> Signed-off-by: Georges Savoundararadj <savoundg at gmail.com>
>> Cc: Denys Vlasenko <vda.linux at googlemail.com>
>> Cc: Timo Teräs <timo.teras at iki.fi>
>> Cc: Dan Fandrich <dan at coneharvesters.com>
>> ---
>>   miscutils/fbsplash.c | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/miscutils/fbsplash.c b/miscutils/fbsplash.c
>> index 3ddf8a2..5e20cdb 100644
>> --- a/miscutils/fbsplash.c
>> +++ b/miscutils/fbsplash.c
>> @@ -151,13 +151,13 @@ static void fb_open(const char *strfb_device)
>>   
>>   	// map the device in memory
>>   	G.addr = mmap(NULL,
>> -			(G.scr_var.yres_virtual ?: G.scr_var.yres) *
>> G.scr_fix.line_length,
>> -			PROT_WRITE, MAP_SHARED, fbfd, 0);
>> +			G.scr_var.yres * G.scr_fix.line_length,
>> +			PROT_WRITE, MAP_SHARED, fbfd,
>> +			// point to the start of the visible screen
>> +			G.scr_var.yoffset * G.scr_fix.line_length +
>> G.scr_var.xoffset * G.bytes_per_pixel); if (G.addr == MAP_FAILED)
>>   		bb_perror_msg_and_die("mmap");
>>   
>> -	// point to the start of the visible screen
>> -	G.addr += G.scr_var.yoffset * G.scr_fix.line_length +
>> G.scr_var.xoffset * G.bytes_per_pixel; close(fbfd);
>>   }
>>