From: Dan Carpenter <dan.carpenter@oracle.com>
To: Hyunwoo Kim <imv4bel@gmail.com>,
deller@gmx.de, linux-fbdev@vger.kernel.org,
Masami Ichikawa <masami.ichikawa@miraclelinux.com>,
cip-dev <cip-dev@lists.cip-project.org>,
Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
Subject: Re: [PATCH] pxa3xx-gcu: Fix integer overflow in pxa3xx_gcu_write
Date: Tue, 20 Sep 2022 09:13:31 +0300 [thread overview]
Message-ID: <YylaC1wHHyLw22D3@kadam> (raw)
In-Reply-To: <YylXes0RnFv97uKU@kili>
On Tue, Sep 20, 2022 at 09:02:34AM +0300, Dan Carpenter wrote:
> On Mon, Jun 20, 2022 at 07:00:10AM -0700, Hyunwoo Kim wrote:
> > In pxa3xx_gcu_write, a count parameter of
> > type size_t is passed to words of type int.
> > Then, copy_from_user may cause a heap overflow because
> > it is used as the third argument of copy_from_user.
> >
> > Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
> > ---
> > drivers/video/fbdev/pxa3xx-gcu.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/video/fbdev/pxa3xx-gcu.c b/drivers/video/fbdev/pxa3xx-gcu.c
> > index 043cc8f9ef1c..c3cd1e1cc01b 100644
> > --- a/drivers/video/fbdev/pxa3xx-gcu.c
> > +++ b/drivers/video/fbdev/pxa3xx-gcu.c
> > @@ -381,7 +381,7 @@ pxa3xx_gcu_write(struct file *file, const char *buff,
> > struct pxa3xx_gcu_batch *buffer;
> > struct pxa3xx_gcu_priv *priv = to_pxa3xx_gcu_priv(file);
> >
> > - int words = count / 4;
> > + size_t words = count / 4;
>
> The count variable is actually capped at MAX_RW_COUNT in vfs_write()
> so "words" cannot be negative. This patch helps clean up the code but
> it does not affect run time.
Btw, the other thing which prevents this from being expliotable is that
if you pass a negative value to copy_from_user() it will not copy
anything because of the check in check_copy_size(). See commit
6d13de1489b6 ("uaccess: disallow > INT_MAX copy sizes").
Linus has sort of gotten annoyed with me before for pointing this stuff
out because it seemed like maybe I wasn't properly grateful to people
auditing the code and fixing bugs. I am grateful. This patch is
totally the correct thing to do. It's just that it's not really
exploitable as described in the commit message.
regards,
dan carpenter
next prev parent reply other threads:[~2022-09-20 6:14 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-20 6:02 [PATCH] pxa3xx-gcu: Fix integer overflow in pxa3xx_gcu_write Dan Carpenter
2022-09-20 6:13 ` Dan Carpenter [this message]
2022-09-20 6:22 ` Hyunwoo Kim
2022-09-20 7:12 ` Dan Carpenter
-- strict thread matches above, loose matches on Subject: below --
2022-06-11 19:28 Hyunwoo Kim
2022-06-20 12:50 ` Helge Deller
2022-06-20 14:17 ` Hyunwoo Kim
2022-06-20 18:13 ` Helge Deller
2022-06-20 18:16 ` Hyunwoo Kim
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YylaC1wHHyLw22D3@kadam \
--to=dan.carpenter@oracle.com \
--cc=cip-dev@lists.cip-project.org \
--cc=deller@gmx.de \
--cc=harshit.m.mogalapalli@oracle.com \
--cc=imv4bel@gmail.com \
--cc=linux-fbdev@vger.kernel.org \
--cc=masami.ichikawa@miraclelinux.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).