Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 3 Feb 2022 10:18:56 +0100 (CET)
From:   Richard Weinberger <richard@nod.at>
To:     =?utf-8?B?TWljaGHFgiBLxJlwaWXFhA==?= <kernel@kempniu.pl>
Cc:     Miquel Raynal <miquel.raynal@bootlin.com>,
        Vignesh Raghavendra <vigneshr@ti.com>,
        Boris Brezillon <boris.brezillon@collabora.com>,
        linux-mtd <linux-mtd@lists.infradead.org>,
        linux-kernel <linux-kernel@vger.kernel.org>
Message-ID: <1173246756.12597.1643879936765.JavaMail.zimbra@nod.at>
In-Reply-To: <20220125104822.8420-5-kernel@kempniu.pl>
References: <20220125104822.8420-1-kernel@kempniu.pl> <20220125104822.8420-5-kernel@kempniu.pl>
Subject: Re: [PATCH v3 4/4] mtdchar: add MEMREAD ioctl
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8BIT
Thread-Topic: mtdchar: add MEMREAD ioctl
Thread-Index: 4v2g4Z4lx0v5vFZ8+gLXLp2J71QhHg==
Precedence: bulk

Michał,

----- Ursprüngliche Mail -----
> Von: "Michał Kępień" <kernel@kempniu.pl>
> An: "Miquel Raynal" <miquel.raynal@bootlin.com>, "richard" <richard@nod.at>, "Vignesh Raghavendra" <vigneshr@ti.com>
> CC: "Boris Brezillon" <boris.brezillon@collabora.com>, "linux-mtd" <linux-mtd@lists.infradead.org>, "linux-kernel"
> <linux-kernel@vger.kernel.org>
> Gesendet: Dienstag, 25. Januar 2022 11:48:22
> Betreff: [PATCH v3 4/4] mtdchar: add MEMREAD ioctl

> +	if (req.start + req.len > mtd->size) {

I think this can overflow since both req.start and req.len are u64.
So an evil-doer might bypass this check.

> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	datbuf_len = min_t(size_t, req.len, mtd->erasesize);
> +	if (datbuf_len > 0) {
> +		datbuf = kmalloc(datbuf_len, GFP_KERNEL);

If mtd->erasesize is large (which is not uncommon these days) you might
request more from kmalloc() than it can serve.
Maybe kvmalloc() makes more sense?

> +		if (!datbuf) {
> +			ret = -ENOMEM;
> +			goto out;
> +		}
> +	}
> +
> +	oobbuf_len = min_t(size_t, req.ooblen, mtd->erasesize);
> +	if (oobbuf_len > 0) {
> +		oobbuf = kmalloc(oobbuf_len, GFP_KERNEL);

Same.

Thanks,
//richard