lkml.org 
[lkml]   [2022]   [Feb]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] fbdev: fbmem: Fix the implicit type casting
    Hi Helge,

    On Tue, Feb 01, 2022 at 04:02:40PM +0100, Helge Deller wrote:
    > On 1/31/22 07:57, Yizhuo Zhai wrote:
    > > In function do_fb_ioctl(), the "arg" is the type of unsigned long,
    >
    > yes, because it comes from the ioctl framework...
    >
    > > and in "case FBIOBLANK:" this argument is casted into an int before
    > > passig to fb_blank().
    >
    > which makes sense IMHO.
    >
    > > In fb_blank(), the comparision if (blank > FB_BLANK_POWERDOWN) would
    > > be bypass if the original "arg" is a large number, which is possible
    > > because it comes from the user input.
    >
    > The main problem I see with your patch is that you change the behaviour.
    > Let's assume someone passes in -1UL.
    > With your patch applied, this means the -1 (which is e.g. 0xffffffff on 32bit)
    > is converted to a positive integer and will be capped to FB_BLANK_POWERDOWN.
    > Since most blank functions just check and react on specific values, you changed
    > the behaviour that the screen now gets blanked at -1, while it wasn't before.
    >
    > One could now argue, that it's undefined behaviour if people
    > pass in wrong values, but anyway, it's different now.

    We should just plug this hole and in case an illegal value is passed
    then return -EINVAL.

    Acceptable values are FB_BLANK_UNBLANK..FB_BLANK_POWERDOWN,
    anything less than or greater than should result in -EINVAL.

    We miss this kind or early checks in many places, and we see the effect
    of this in some drivers where they do it locally for no good.

    Sam

    \
     
     \ /
      Last update: 2022-02-05 06:45    [W:4.286 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site