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] [thread-next>] [day] [month] [year] [list]

Message-ID: <fabbceb3-90a3-2cc8-c664-e53fcd504393@huawei.com>
Date: Mon, 9 Dec 2024 14:27:58 +0800
From: Zhihao Cheng <chengzhihao1@...wei.com>
To: Dan Carpenter <dan.carpenter@...aro.org>
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

在 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?
> 
> Which commit should get the fixes tag?
> 
> I should have added a CC to the stable tree though.  I did that correctly
> in an earlier draft of this patch but I messed up in this version. :/
> 
> regards,
> dan carpenter
> 
> .
>