linux-kernel - Re: [PATCH] mtdchar: fix integer overflow in read/write ioctls

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <0bc9812c-135a-49b8-aed8-67ee4e3d6d6d@stanley.mountain>
Date: Mon, 9 Dec 2024 10:37:34 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Zhihao Cheng <chengzhihao1@...wei.com>
Cc: Michał Kępień <kernel@...pniu.pl>,
	Miquel Raynal <miquel.raynal@...tlin.com>,
	Richard Weinberger <richard@....at>,
	Vignesh Raghavendra <vigneshr@...com>,
	linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org,
	kernel-janitors@...r.kernel.org
Subject: Re: [PATCH] mtdchar: fix integer overflow in read/write ioctls

On Mon, Dec 09, 2024 at 02:27:58PM +0800, Zhihao Cheng wrote:
> 在 2024/12/8 1:05, Dan Carpenter 写道:
> > On Sat, Dec 07, 2024 at 12:17:33PM +0800, Zhihao Cheng wrote:
> > > 在 2024/12/7 4:26, Dan Carpenter 写道:
> > > > The "req.start" and "req.len" variables are u64 values that come from the
> > > > user at the start of the function.  We mask away the high 32 bits of
> > > > "req.len" so that's capped at U32_MAX but the "req.start" variable can go
> > > > up to U64_MAX.
> > > > 
> > > > Use check_add_overflow() to fix this bug.
> > > > 
> > > > Fixes: 6420ac0af95d ("mtdchar: prevent unbounded allocation in MEMWRITE ioctl")
> > > 
> > > Hi, Dan. Why this fix tag? I think the adding result('req.start' and
> > > 'req.len') could be overflow too before this commit.
> > > 
> > 
> > I've looked at this again, and I still don't see the bug before the
> > commit.  Secondly, commit a1eda864c04c ("mtdchar: prevent integer
> > overflow in a safety check") is missing a Fixes tag but the message says
> > that it's this commit which introduced the bug.
> 
> Ah, I see. There is not an addition operation for 'req.start' and 'req.len'
> until commit 6420ac0af95d("mtdchar: prevent unbounded allocation in MEMWRITE
> ioctl") and 095bb6e44eb1("mtdchar: add MEMREAD ioctl"), so I guess the there
> should be two fix tags?

Ah, sure.  I can tag both those commits.

regards,
dan carpenter