Le 22/04/2025 à 20:23, SeongJae Park a écrit :
On Tue, 22 Apr 2025 13:44:39 +0300 Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
On Tue, Apr 22, 2025 at 01:38:05PM +0300, Dan Carpenter wrote:
On Mon, Apr 21, 2025 at 02:24:24PM +0800, Su Hui wrote:
It's safer to using kmalloc_array() and size_add() because it can
prevent possible overflow problem.
Signed-off-by: Su Hui <suhui@xxxxxxxxxxxx>
[...]
--- a/mm/damon/sysfs-schemes.c
+++ b/mm/damon/sysfs-schemes.c
@@ -465,7 +465,8 @@ static ssize_t memcg_path_store(struct kobject *kobj,
{
struct damon_sysfs_scheme_filter *filter = container_of(kobj,
struct damon_sysfs_scheme_filter, kobj);
- char *path = kmalloc(sizeof(*path) * (count + 1), GFP_KERNEL);
+ char *path = kmalloc_array(size_add(count, 1), sizeof(*path),
+ GFP_KERNEL);
Count is clamped in rw_verify_area().
Smatch does a kind of ugly hack to handle rw_verify_area() which is that
it says neither the count nor the pos can be more than 1G. And obviously
files which are larger than 2GB exist but pretending they don't silences
all these integer overflow warnings.
Actually rw_verify_area() ensures that "pos + count" can't overflow. But
here we are multiplying. Fortunately, we are multiplying by 1 so that's
safe and also count can't be larger than PAGE_SIZE here which is safe as
well.
Thank you for adding these details, Dan. I understand the size_add() change
can make warnings slience, though it is not really fixing a real bug. So I
believe there is no action item to make a change to this patch. Maybe making
the commit message more clarified can be helpful, though?
Please let me know if I'm misunderstanding your point and/or you want some
changes.
As sizeof(*path) = 1, maybe, just change it to:
char *path = kmalloc(count + 1, GFP_KERNEL);
CJ
Thanks,
SJ
regards,
dan carpenter
