Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <CAMhUBjmFhqTLBscHHVZ1VTSqrJBT1VEevA+KkjY+y9_ZtdRkMg@mail.gmail.com>
 <631f03bd-0fdf-9cc8-bf37-89235fb84162@gmx.de>
In-Reply-To: <631f03bd-0fdf-9cc8-bf37-89235fb84162@gmx.de>
From:   Geert Uytterhoeven <geert@linux-m68k.org>
Date:   Mon, 4 Apr 2022 13:46:16 +0200
Message-ID: <CAMuHMdUiEo8q9x0C0x5zOM=ax1=S06=s0JjcJvZYD4aMGLmEaQ@mail.gmail.com>
Subject: =?UTF-8?B?UmU6IFtCVUddIGZiZGV2OiBpNzQwZmI6IERpdmlkZSBlcnJvciB3aGVuIOKAmHZhci0+cA==?=
        =?UTF-8?B?aXhjbG9ja+KAmSBpcyB6ZXJv?=
To:     Helge Deller <deller@gmx.de>
Cc:     Zheyu Ma <zheyuma97@gmail.com>,
        Linux Fbdev development list <linux-fbdev@vger.kernel.org>,
        DRI Development <dri-devel@lists.freedesktop.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Helge,

On Sun, Apr 3, 2022 at 5:41 PM Helge Deller <deller@gmx.de> wrote:
> On 4/3/22 13:26, Zheyu Ma wrote:
> > I found a bug in the function i740fb_set_par().
>
> Nice catch!
>
> > When the user calls the ioctl system call without setting the value to
> > 'var->pixclock', the driver will throw a divide error.
> >
> > This bug occurs because the driver uses the value of 'var->pixclock'
> > without checking it, as the following code snippet show:
> >
> > if ((1000000 / var->pixclock) > DACSPEED8) {
> >      dev_err(info->device, "requested pixclock %i MHz out of range
> > (max. %i MHz at 8bpp)\n",
> >          1000000 / var->pixclock, DACSPEED8);
> >     return -EINVAL;x
> > }
> >
> > We can fix this by checking the value of 'var->pixclock' in the
> > function i740fb_check_var() similar to commit
> > b36b242d4b8ea178f7fd038965e3cac7f30c3f09, or we should set the lowest
> > supported value when this field is zero.
> > I have no idea about which solution is better.
>
> Me neither.
> I think a solution like commit b36b242d4b8ea178f7fd038965e3cac7f30c3f09
> is sufficient.
>
> Note that i740fb_set_par() is called in i740fb_resume() as well.
> Since this doesn't comes form userspace I think adding a check for
> the return value there isn't necessary.
>
> Would you mind sending a patch like b36b242d4b8ea178f7fd038965e3cac7f30c3f09 ?

When passed an invalid value, .check_var() is supposed to
round up the invalid to a valid value, if possible.

Commit b36b242d4b8ea178 ("video: fbdev: asiliantfb: Error out if
'pixclock' equals zero") does not do that.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds