From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 06DE1C432BE for ; Mon, 30 Aug 2021 14:30:54 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E42786023E for ; Mon, 30 Aug 2021 14:30:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237229AbhH3Obq (ORCPT ); Mon, 30 Aug 2021 10:31:46 -0400 Received: from mail-vs1-f44.google.com ([209.85.217.44]:40615 "EHLO mail-vs1-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237181AbhH3Obm (ORCPT ); Mon, 30 Aug 2021 10:31:42 -0400 Received: by mail-vs1-f44.google.com with SMTP id d6so4300973vsr.7; Mon, 30 Aug 2021 07:30:48 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=VgKhHizq2L8dQZoQcxBfufERxFjnVK3q48k2GBzHxbg=; b=gHEekl3GLlnCmOd+FZFDoDl80NkQfMka975uFWQjgPFQ/lNnGYXkXtGwCXC+xGsnsf pJE5ClOj7MEyDLhYoiKDNwPd/IZJb0OQ9uRDsUhS1vEayYV5cKkNrUy8wh0xO4N4M3kg K3AKMSkO7mWf8JSNCqz/T7LyiWCgRair8f9pklBcm+1LNKsbhBMlBDfpJnzA3MJwlmaY 2wSnWm6QxR+H0mWlKEZ6Jel8GV4/Su/saBWUpH/Y3wCzbNjSS0I2hiIEkEFaJ6ByibOo uWsY6zVEnYKpSsUYy5toRpnpaAs1f5/U+KOJZBKdxQ3RdFjuk4l0zVT6oTKcJS9ZmaOL YjGw== X-Gm-Message-State: AOAM530ZRiWtKnVF1w4BQYd8QrFG/6FsJiAODizUFT9VD9KgiQlTajIp 7nYKOUeICmS67fm92WSWsu40YRMjHRnzNHsDN/c= X-Google-Smtp-Source: ABdhPJwXrGuINMzmLOeLoNvKrXr8kded9JMrOcZu3G8T8pbvFnWwxXU9kXJU7O9Mej5MH/LuttPsiK2qv0RDHBrX1gw= X-Received: by 2002:a67:cb0a:: with SMTP id b10mr14980584vsl.9.1630333847661; Mon, 30 Aug 2021 07:30:47 -0700 (PDT) MIME-Version: 1.0 References: <000000000000815b9605c70e74f8@google.com> <131b24e5-ee31-6f7b-42b4-c34583711913@infradead.org> <2fccb5d3-191c-924e-159f-1c9d423e282f@i-love.sakura.ne.jp> <20210830130000.GW7722@kadam> <8ed0ca59-226b-2d0e-b1ae-82305202558d@i-love.sakura.ne.jp> <20210830134719.GI12231@kadam> <03d0f549-9731-8b06-1393-60d4bef27884@i-love.sakura.ne.jp> In-Reply-To: <03d0f549-9731-8b06-1393-60d4bef27884@i-love.sakura.ne.jp> From: Geert Uytterhoeven Date: Mon, 30 Aug 2021 16:30:36 +0200 Message-ID: Subject: Re: [syzbot] BUG: unable to handle kernel paging request in vga16fb_fillrect To: Tetsuo Handa Cc: Dan Carpenter , Randy Dunlap , syzbot , Andrew Morton , Bartlomiej Zolnierkiewicz , Colin King , DRI Development , Linux Fbdev development list , Linux Kernel Mailing List , Masahiro Yamada , syzkaller-bugs@googlegroups.com Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Tetsuo, On Mon, Aug 30, 2021 at 4:26 PM Tetsuo Handa wrote: > On 2021/08/30 22:47, Dan Carpenter wrote: > > On Mon, Aug 30, 2021 at 10:37:31PM +0900, Tetsuo Handa wrote: > >> On 2021/08/30 22:00, Dan Carpenter wrote: > >>>>> diff --git a/drivers/video/fbdev/vga16fb.c b/drivers/video/fbdev/vga16fb.c > >>>>> index e2757ff1c23d..e483a3f5fd47 100644 > >>>>> --- a/drivers/video/fbdev/vga16fb.c > >>>>> +++ b/drivers/video/fbdev/vga16fb.c > >>>>> @@ -403,7 +403,7 @@ static int vga16fb_check_var(struct fb_var_screeninfo *var, > >>>>> > >>>>> if (yres > vyres) > >>>>> vyres = yres; > >>>>> - if (vxres * vyres > maxmem) { > >>>>> + if ((u64) vxres * vyres > (u64) maxmem) { > >>>> > >>>> Mindlessly changing the sizes is not the solution. > >>>> Please use e.g. the array_size() helper from > >>>> instead. > >>> > >>> On a 64bit system the array_size() macro is going to do the exact same > >>> casts? But I do think this code would be easier to understand if the > >>> integer overflow check were pull out separately and done first: > >>> > >>> if (array_size(vxres, vyres) >= UINT_MAX) > >>> return -EINVAL; > >> > >> This is wrong. array_size() returns ULONG_MAX on 64bits upon overflow and > >> returns UINT_MAX on 32bits upon overflow. However, UINT_MAX is a valid > >> value without overflow (e.g. vxres == UINT_MAX / 15 && vyres == 15). > > > > Huh... I just assumed we didn't allow resolutions that high. > > Of course, we don't allow resolutions that high. ;-) > > Since I don't know possible max resolutions, I chose UINT_MAX + 1 as a common > limit for returning -EINVAL. Unless overflow happens, vga16fb_check_var() will > return -ENOMEM on such high resolutions. The highest possible value of maxmem inside vga16fb_check_var() is 65536. So I believe if (array_size(vxres, vyres) > maxmem) should work fine. 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1F5B3C432BE for ; Mon, 30 Aug 2021 14:30:51 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id D3ED260F4B for ; Mon, 30 Aug 2021 14:30:50 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org D3ED260F4B Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=linux-m68k.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 4839B89E57; Mon, 30 Aug 2021 14:30:50 +0000 (UTC) Received: from mail-vs1-f47.google.com (mail-vs1-f47.google.com [209.85.217.47]) by gabe.freedesktop.org (Postfix) with ESMTPS id 1E5A689E57 for ; Mon, 30 Aug 2021 14:30:49 +0000 (UTC) Received: by mail-vs1-f47.google.com with SMTP id f6so8093235vsr.3 for ; Mon, 30 Aug 2021 07:30:49 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=VgKhHizq2L8dQZoQcxBfufERxFjnVK3q48k2GBzHxbg=; b=UTnFQJIZFgWypJntdakjFYd/UFYcv/GPsPofenRAWn/6gDCx67R3XUmc9Du2Mw+9gI /yy74u47cCGc100kv8DtaTM0JuLs2oeExXx8+abOfGgFFhhim+Ns7ic/YIlt+UbEYdpL DQFszsmocHTsoJOqQQeV0noKxwicsX8mjFnd4mHUlQXx2PU1C0FP7mAST/xRqU4OlGTl yTqiYPlbYkEVhSRQVE0J2pgPwOiFdE3K28lvI2lwuZUVkv7+2GFbhWoUgIpy7gi+PKGk WHKU02WvWXo7+h9haK8VuZRPygCYgSqSDrMLfMuQ9ZSYzVXF7Qy16QWyKQr21OHOly2k W6FA== X-Gm-Message-State: AOAM530oXPwR2a9mA9INfJHTYkjT74+i7VWfV8ljIC1wODakiux/SYyM S+SMJZafpg/2Rha6AEQrtBOA4/TE7w6PU5+It/g= X-Google-Smtp-Source: ABdhPJwXrGuINMzmLOeLoNvKrXr8kded9JMrOcZu3G8T8pbvFnWwxXU9kXJU7O9Mej5MH/LuttPsiK2qv0RDHBrX1gw= X-Received: by 2002:a67:cb0a:: with SMTP id b10mr14980584vsl.9.1630333847661; Mon, 30 Aug 2021 07:30:47 -0700 (PDT) MIME-Version: 1.0 References: <000000000000815b9605c70e74f8@google.com> <131b24e5-ee31-6f7b-42b4-c34583711913@infradead.org> <2fccb5d3-191c-924e-159f-1c9d423e282f@i-love.sakura.ne.jp> <20210830130000.GW7722@kadam> <8ed0ca59-226b-2d0e-b1ae-82305202558d@i-love.sakura.ne.jp> <20210830134719.GI12231@kadam> <03d0f549-9731-8b06-1393-60d4bef27884@i-love.sakura.ne.jp> In-Reply-To: <03d0f549-9731-8b06-1393-60d4bef27884@i-love.sakura.ne.jp> From: Geert Uytterhoeven Date: Mon, 30 Aug 2021 16:30:36 +0200 Message-ID: Subject: Re: [syzbot] BUG: unable to handle kernel paging request in vga16fb_fillrect To: Tetsuo Handa Cc: Dan Carpenter , Randy Dunlap , syzbot , Andrew Morton , Bartlomiej Zolnierkiewicz , Colin King , DRI Development , Linux Fbdev development list , Linux Kernel Mailing List , Masahiro Yamada , syzkaller-bugs@googlegroups.com Content-Type: text/plain; charset="UTF-8" X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Hi Tetsuo, On Mon, Aug 30, 2021 at 4:26 PM Tetsuo Handa wrote: > On 2021/08/30 22:47, Dan Carpenter wrote: > > On Mon, Aug 30, 2021 at 10:37:31PM +0900, Tetsuo Handa wrote: > >> On 2021/08/30 22:00, Dan Carpenter wrote: > >>>>> diff --git a/drivers/video/fbdev/vga16fb.c b/drivers/video/fbdev/vga16fb.c > >>>>> index e2757ff1c23d..e483a3f5fd47 100644 > >>>>> --- a/drivers/video/fbdev/vga16fb.c > >>>>> +++ b/drivers/video/fbdev/vga16fb.c > >>>>> @@ -403,7 +403,7 @@ static int vga16fb_check_var(struct fb_var_screeninfo *var, > >>>>> > >>>>> if (yres > vyres) > >>>>> vyres = yres; > >>>>> - if (vxres * vyres > maxmem) { > >>>>> + if ((u64) vxres * vyres > (u64) maxmem) { > >>>> > >>>> Mindlessly changing the sizes is not the solution. > >>>> Please use e.g. the array_size() helper from > >>>> instead. > >>> > >>> On a 64bit system the array_size() macro is going to do the exact same > >>> casts? But I do think this code would be easier to understand if the > >>> integer overflow check were pull out separately and done first: > >>> > >>> if (array_size(vxres, vyres) >= UINT_MAX) > >>> return -EINVAL; > >> > >> This is wrong. array_size() returns ULONG_MAX on 64bits upon overflow and > >> returns UINT_MAX on 32bits upon overflow. However, UINT_MAX is a valid > >> value without overflow (e.g. vxres == UINT_MAX / 15 && vyres == 15). > > > > Huh... I just assumed we didn't allow resolutions that high. > > Of course, we don't allow resolutions that high. ;-) > > Since I don't know possible max resolutions, I chose UINT_MAX + 1 as a common > limit for returning -EINVAL. Unless overflow happens, vga16fb_check_var() will > return -ENOMEM on such high resolutions. The highest possible value of maxmem inside vga16fb_check_var() is 65536. So I believe if (array_size(vxres, vyres) > maxmem) should work fine. 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