[PATCH] fbsplash: Fix mmap size and offset calculations

[Georges Savoundararadj]

On 05/11/2016 01:16 AM, Timo Teras wrote:
> On Tue, 10 May 2016 23:46:30 -0700
> Georges Savoundararadj <savoundg at gmail.com> wrote:
>
>> 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?
> If you just map the whole memory, you can keep the calculations in one
> place. Splitting it to two place takes more code, is harder to
> understand, and results in larger binary size.
OK, I understand it.
> Mapping more video memory is not harmful. It does not normally take
> memory more (in worst case the mmu page table can grow slightly).
OK
> So what is the benefit of such change? Is there some non-obvious bug it
> is meant to fix? Could you describe it.
>
It seems that there is no benefit of such change.

In the stable versions of busybox, I fall on a segfault because your 
patch was not applied and I fixed the bug that way.

Please disregard this patch.

Thanks,

Georges