From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 76022C433EF for ; Fri, 26 Nov 2021 09:32:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=+JH5P2vZvECLiadg1k9CsRakZ97sLbdHIcBySJdWQj0=; b=qCjiLIiQy1a8z9 pRXNIP82X0a6zUF5O0oeSeiaDst9Z3NoXv3IvQk04wRyddiLIru8n+/U8dErse5sHpZj1Onou+p9B UEvoRigIfVa8FZ/qzknVxbQyk66YEno8ueoKdjMMuKgkN95t1oQmDtY0ieBP6gshen+1Oh4R5FR5r v9Cnnq+ZOOU31VQ+wjnrNFtcs9nWbwSTBhrtEj6KYB+p3zZZW1v07acperUeBnm7wdpDqTrS95HVW 2Hy+xUCcGQH+ZaN8dwvA/1Mi1dYw5tVrwSwHuYfrY6Do7H5rhoCsF9k0qKNGUmC+JJC3/WVfNPngM LiBAwB45sSqK4hVIz77Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mqXZo-009j4t-On; Fri, 26 Nov 2021 09:31:28 +0000 Received: from relay7-d.mail.gandi.net ([217.70.183.200]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mqXZk-009j4H-Rg for linux-mtd@lists.infradead.org; Fri, 26 Nov 2021 09:31:27 +0000 Received: (Authenticated sender: miquel.raynal@bootlin.com) by relay7-d.mail.gandi.net (Postfix) with ESMTPSA id 4DE162000E; Fri, 26 Nov 2021 09:31:17 +0000 (UTC) Date: Fri, 26 Nov 2021 10:31:16 +0100 From: Miquel Raynal To: =?UTF-8?B?TWljaGHFgiBLxJlwaWXFhA==?= Cc: Richard Weinberger , Vignesh Raghavendra , Boris Brezillon , linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] mtdchar: prevent unbounded allocation in MEMWRITE ioctl Message-ID: <20211126103116.5bef6bc0@xps13> In-Reply-To: References: <20211025082104.8017-1-kernel@kempniu.pl> <20211122103122.424326a1@xps13> Organization: Bootlin X-Mailer: Claws Mail 3.17.7 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211126_013125_219880_FB9A8A01 X-CRM114-Status: GOOD ( 48.29 ) X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Sender: "linux-mtd" Errors-To: linux-mtd-bounces+linux-mtd=archiver.kernel.org@lists.infradead.org SGkgTWljaGHFgiwKCmtlcm5lbEBrZW1wbml1LnBsIHdyb3RlIG9uIFRodSwgMjUgTm92IDIwMjEg MTM6MDg6MTYgKzAxMDA6Cgo+IEhpIE1pcXXDqGwsCj4gCj4gVEw7RFI6IHRoYW5rcywgeW91ciBj b21tZW50IG1hZGUgbWUgbG9vayBjbG9zZXIgYW5kIEkgZm91bmQgd2hhdCBzZWVtcwo+IHRvIGJl IGEgZmVhc2libGUgd29ya2Fyb3VuZCB0aGF0IEkgd2lsbCBhcHBseSBpbiB2MiAoYWxvbmcgb3Ro ZXIgZml4ZXMpLgo+IAo+ID4gPiBEZXNwaXRlIG15IGVmZm9ydHMsIHRoZSBwYXRjaCBkb2VzIF9u b3RfIHJldGFpbiBhYnNvbHV0ZSBiYWNrd2FyZAo+ID4gPiBjb21wYXRpYmlsaXR5IGFuZCBJIGRv IG5vdCBrbm93IHdoZXRoZXIgdGhpcyBpcyBhY2NlcHRhYmxlLgo+ID4gPiBTcGVjaWZpY2FsbHks IG11bHRpLWVyYXNlYmxvY2stc2l6ZWQgd3JpdGVzIChyZXF1aXJpbmcgbXVsdGlwbGUgbG9vcAo+ ID4gPiBpdGVyYXRpb25zIHRvIGJlIHByb2Nlc3NlZCkgd2hpY2ggc3VjY2VlZGVkIHdpdGggdGhl IG9yaWdpbmFsIGNvZGUgX21heV8KPiA+ID4gbm93IHJldHVybiBlcnJvcnMgYW5kL29yIHdyaXRl IGRpZmZlcmVudCBjb250ZW50cyB0byB0aGUgZGV2aWNlIHRoYW4gdGhlCj4gPiA+IG9yaWdpbmFs IGNvZGUsIGRlcGVuZGluZyBvbiB0aGUgTVREIG1vZGUgb2Ygb3BlcmF0aW9uIHJlcXVlc3RlZCBh bmQgb24KPiA+ID4gd2hldGhlciB0aGUgc3RhcnQgb2Zmc2V0IGlzIHBhZ2UtYWxpZ25lZC4gIFRo ZSBkb2N1bWVudGF0aW9uIGZvciBzdHJ1Y3QKPiA+ID4gbXRkX29vYl9vcHMgd2FybnMgYWJvdXQg ZXZlbiBtdWx0aS1wYWdlIHdyaXRlIHJlcXVlc3RzLCBidXQuLi4KPiA+ID4gCj4gPiA+IEV4YW1w bGU6Cj4gPiA+IAo+ID4gPiAgICAgTVREIGRldmljZSBwYXJhbWV0ZXJzOgo+ID4gPiAKPiA+ID4g ICAgICAgLSBtdGQtPndyaXRlc2l6ZSA9IDIwNDgKPiA+ID4gICAgICAgLSBtdGQtPmVyYXNlc2l6 ZSA9IDEzMTA3Mgo+ID4gPiAgICAgICAtIDY0IGJ5dGVzIG9mIHJhdyBPT0Igc3BhY2UgcGVyIHBh Z2UKPiA+ID4gCj4gPiA+ICAgICBzdHJ1Y3QgbXRkX3dyaXRlX3JlcSByZXEgPSB7Cj4gPiA+ICAg ICAgICAgLnN0YXJ0ID0gMjA0OCwKPiA+ID4gICAgICAgICAubGVuID0gMjYyMTQ0LAo+ID4gPiAg ICAgICAgIC5vb2JsZW4gPSA2NCwKPiA+ID4gICAgICAgICAudXNyX2RhdGEgPSAuLi4sCj4gPiA+ ICAgICAgICAgLnVzcl9vb2IgPSAuLi4sCj4gPiA+ICAgICAgICAgLm1vZGUgPSBNVERfT1BTX1JB VywKPiA+ID4gICAgIH07Cj4gPiA+IAo+ID4gPiAgICAgKFRoaXMgaXMgYSAxMjgtcGFnZSB3cml0 ZSB3aXRoIE9PQiBkYXRhIHN1cHBsaWVkIGZvciBqdXN0IG9uZSBwYWdlLikKPiA+ID4gCj4gPiA+ ICAgICBDdXJyZW50IG10ZGNoYXJfd3JpdGVfaW9jdGwoKSByZXR1cm5zIDAgZm9yIHRoaXMgcmVx dWVzdCBhbmQgd3JpdGVzCj4gPiA+ICAgICAxMjggcGFnZXMgb2YgZGF0YSBhbmQgMSBwYWdlJ3Mg d29ydGggb2YgT09CIGRhdGEgKDY0IGJ5dGVzKSB0byB0aGUKPiA+ID4gICAgIE1URCBkZXZpY2Uu Cj4gPiA+IAo+ID4gPiAgICAgUGF0Y2hlZCBtdGRjaGFyX3dyaXRlX2lvY3RsKCkgbWF5IHJldHVy biBhbiBlcnJvciBiZWNhdXNlIHRoZQo+ID4gPiAgICAgb3JpZ2luYWwgcmVxdWVzdCBnZXRzIHNw bGl0IGludG8gdHdvIGNodW5rcyAoPGRhdGFfbGVuLCBvb2JfbGVuPik6Cj4gPiA+IAo+ID4gPiAg ICAgICAgIDwxMzEwNzIsIDY0Pgo+ID4gPiAgICAgICAgIDwxMzEwNzIsIDA+Cj4gPiA+IAo+ID4g PiAgICAgYW5kIHRoZSBzZWNvbmQgY2h1bmsgd2l0aCB6ZXJvIE9PQiBkYXRhIGxlbmd0aCBtYXkg bWFrZSB0aGUgTVRECj4gPiA+ICAgICBkcml2ZXIgdW5oYXBweSBpbiByYXcgbW9kZSAocmVzdWx0 aW5nIGluIG9ubHkgdGhlIGZpcnN0IDY0IHBhZ2VzIG9mCj4gPiA+ICAgICBkYXRhIGFuZCAxIHBh Z2UncyB3b3J0aCBvZiBPT0IgZGF0YSBnZXR0aW5nIHdyaXR0ZW4gdG8gdGhlIE1URAo+ID4gPiAg ICAgZGV2aWNlKS4gIAo+ID4gCj4gPiBJc24ndCB0aGlzIGEgZHJpdmVyIGlzc3VlIGluc3RlYWQ/ IEkgbWVhbiwgd3JpdGluZyBhbiBlcmFzZWJsb2NrCj4gPiB3aXRob3V0IHByb3ZpZGluZyBhbnkg T09CIGRhdGEgaXMgY29tcGxldGVseSBmaW5lLCBpZiB0aGUgZHJpdmVyCj4gPiBhY2NlcHRzIDIg YmxvY2tzICsgMSBwYWdlIE9PQiBidXQgcmVmdXNlcyAxIGJsb2NrICsgMSBwYWdlIE9PQiBhbmQg dGhlbgo+ID4gMSBibG9jaywgaXQncyBicm9rZW4sIG5vPyBIYXZlIHlvdSBleHBlcmllbmNlZCBz dWNoIGEgc2l0dWF0aW9uIGluIHlvdXIKPiA+IHRlc3Rpbmc/ICAKPiAKPiBJIG1heSBub3QgaGF2 ZSBleHByZXNzZWQgbXlzZWxmIGNsZWFybHkgaGVyZSwgc29ycnkgLSB0aGUgZXhhbXBsZSB3YXMK PiBhbHJlYWR5IGdldHRpbmcgYSBiaXQgbGVuZ3RoeSBhdCB0aGF0IHBvaW50Li4uIDopCj4gCj4g SSB0ZXN0ZWQgdGhlIHBhdGNoIHdpdGggbmFuZHNpbSwgYnV0IEkgZG8gbm90IHRoaW5rIGl0IGlz IHRoYXQgc3BlY2lmaWMKPiBkcml2ZXIgdGhhdCBpcyBicm9rZW4uICBUaGUgY2F0Y2ggaXMgdGhh dCB3aGVuIG10ZF93cml0ZV9vb2IoKSBpcwo+IGNhbGxlZCwgbmFuZF9kb193cml0ZV9vcHMoKSBz cGxpdHMgbXVsdGktcGFnZSByZXF1ZXN0cyBpbnRvIHNpbmdsZS1wYWdlCj4gcmVxdWVzdHMgYW5k IHdoYXQgaXQgcGFzc2VzIHRvIG5hbmRfd3JpdGVfcGFnZSgpIGRlcGVuZHMgb24gd2hldGhlciB0 aGUKPiBzdHJ1Y3QgbXRkX29vYl9vcHMgaXQgd2FzIGludm9rZWQgd2l0aCBoYXMgdGhlIG9vYmJ1 ZiBmaWVsZCBzZXQgdG8gTlVMTAo+IG9yIG5vdC4gIFRoaXMgaXMgb2theSBpbiBpdHNlbGYsIGJ1 dCB3aGVuIGFub3RoZXIgcmVxdWVzdC1zcGxpdHRpbmcKPiAibGF5ZXIiIGlzIGludHJvZHVjZWQg YnkgbXkgcGF0Y2gsIHRoZSBpb2N0bCBtYXkgc3RhcnQgcmV0dXJuaW5nCj4gZGlmZmVyZW50IHJl c3VsdCBjb2RlcyB0aGFuIGl0IHVzZWQgdG8uCj4gCj4gSGVyZSBpcyB3aGF0IGhhcHBlbnMgd2l0 aCB0aGUgdW5wYXRjaGVkIGNvZGUgZm9yIHRoZSBleGFtcGxlIGFib3ZlOgo+IAo+ICAxLiBtdGRj aGFyX3dyaXRlX2lvY3RsKCkgZ2V0cyBjYWxsZWQgd2l0aCB0aGUgZm9sbG93aW5nIHJlcXVlc3Q6 Cj4gCj4gICAgIHN0cnVjdCBtdGRfd3JpdGVfcmVxIHJlcSA9IHsKPiAgICAgICAgIC5zdGFydCA9 IDIwNDgsCj4gICAgICAgICAubGVuID0gMjYyMTQ0LAo+ICAgICAgICAgLm9vYmxlbiA9IDY0LAo+ ICAgICAgICAgLnVzcl9kYXRhID0gMHgxMDAwMDAwMCwKPiAgICAgICAgIC51c3Jfb29iID0gMHgy MDAwMDAwMCwKPiAgICAgICAgIC5tb2RlID0gTVREX09QU19SQVcsCj4gICAgIH07Cj4gCj4gIDIu IFRoZSBhYm92ZSByZXF1ZXN0IGlzIHBhc3NlZCB0aHJvdWdoIHRvIG10ZF93cml0ZV9vb2IoKSB2 ZXJiYXRpbToKPiAKPiAgICAgc3RydWN0IG10ZF9vb2Jfb3BzIG9wcyA9IHsKPiAgICAgICAgIC5t b2RlID0gTVREX09QU19SQVcsCj4gCS5sZW4gPSAyNjIxNDQsCj4gCS5vb2JsZW4gPSA2NCwKPiAg ICAgICAgIC5kYXRidWYgPSAweDEwMDAwMDAwLAo+ICAgICAgICAgLm9vYmJ1ZiA9IDB4MjAwMDAw MDAsCj4gICAgIH07Cj4gCj4gIDMuIG5hbmRfZG9fd3JpdGVfb3BzKCkgc3BsaXRzIHRoaXMgcmVx dWVzdCB1cCBpbnRvIHBhZ2Utc2l6ZWQgcmVxdWVzdHMuCj4gCj4gICAgICBhKSBGb3IgdGhlIGZp cnN0IHBhZ2UsIHRoZSBpbml0aWFsIDIwNDggYnl0ZXMgb2YgZGF0YSArIDY0IGJ5dGVzIG9mCj4g ICAgICAgICBPT0IgZGF0YSBhcmUgcGFzc2VkIHRvIG5hbmRfd3JpdGVfcGFnZSgpLgo+IAo+ICAg ICAgYikgRm9yIGVhY2ggc3Vic2VxdWVudCBwYWdlLCBhIDIwNDgtYnl0ZSBjaHVuayBvZiBkYXRh ICsgNjQgYnl0ZXMKPiAgICAgICAgIG9mIDB4ZmYgYnl0ZXMgYXJlIHBhc3NlZCB0byBuYW5kX3dy aXRlX3BhZ2UoKS4KPiAKPiAgICAgU2luY2UgdGhlIG9vYmJ1ZiBmaWVsZCBpbiB0aGUgc3RydWN0 IG10ZF9vb2Jfb3BzIHBhc3NlZCBpcyBub3QgTlVMTCwKPiAgICAgb29iX3JlcXVpcmVkIGlzIHNl dCB0byAxIGZvciBhbGwgbmFuZF93cml0ZV9wYWdlKCkgY2FsbHMuCj4gCj4gIDQuIFRoZSBhYm92 ZSBjYXVzZXMgdGhlIGRyaXZlciB0byByZWNlaXZlIDIxMTIgYnl0ZXMgb2YgZGF0YSBmb3IgZWFj aAo+ICAgICBwYWdlIHdyaXRlLCB3aGljaCByZXN1bHRzIGluIHRoZSBpb2N0bCBiZWluZyBzdWNj ZXNzZnVsLgo+IAo+IEhlcmUgaXMgd2hhdCBoYXBwZW5zIHdpdGggdGhlIHBhdGNoZWQgY29kZToK PiAKPiAgMS4gbXRkY2hhcl93cml0ZV9pb2N0bCgpIGdldHMgY2FsbGVkIHdpdGggdGhlIHNhbWUg cmVxdWVzdCBhcyBhYm92ZS4KPiAKPiAgMi4gVGhlIG9yaWdpbmFsIHJlcXVlc3QgZ2V0cyBzcGxp dCBpbnRvIHR3byBlcmFzZWJsb2NrLXNpemVkCj4gICAgIG10ZF93cml0ZV9vb2IoKSBjYWxsczoK PiAKPiAgICAgIGEpIHN0cnVjdCBtdGRfb29iX29wcyBvcHMgPSB7Cj4gICAgICAgICAgICAgLm1v ZGUgPSBNVERfT1BTX1JBVywKPiAgICAgICAgICAgICAubGVuID0gMTMxMDcyLAo+ICAgICAgICAg ICAgIC5vb2JsZW4gPSA2NCwKPiAgICAgICAgICAgICAuZGF0YnVmID0gMHgxMDAwMDAwMCwKPiAg ICAgICAgICAgICAub29iYnVmID0gMHgyMDAwMDAwMCwKPiAgICAgICAgIH07Cj4gCj4gICAgICBi KSBzdHJ1Y3QgbXRkX29vYl9vcHMgb3BzID0gewo+ICAgICAgICAgICAgIC5tb2RlID0gTVREX09Q U19SQVcsCj4gICAgICAgICAgICAgLmxlbiA9IDEzMTA3MiwKPiAgICAgICAgICAgICAub29ibGVu ID0gMCwKPiAgICAgICAgICAgICAuZGF0YnVmID0gMHgxMDAyMDAwMCwKPiAgICAgICAgICAgICAu b29iYnVmID0gTlVMTCwKPiAgICAgICAgIH07Cj4gCj4gICAgIChNeSBjb2RlIHNldHMgb29iYnVm IHRvIE5VTEwgaWYgb29ibGVuIGlzIDAuKQo+IAo+ICAzLiBuYW5kX2RvX3dyaXRlX29wcygpIHNw bGl0cyB0aGUgZmlyc3QgcmVxdWVzdCB1cCBpbnRvIHBhZ2Utc2l6ZWQKPiAgICAgcmVxdWVzdHMg dGhlIHNhbWUgd2F5IGFzIGZvciB0aGUgb3JpZ2luYWwgY29kZS4gIEl0IHJldHVybnMKPiAgICAg c3VjY2Vzc2Z1bGx5LCBzbyBtdGRjaGFyX3dyaXRlX2lvY3RsKCkgcHJvY2VlZHMgd2l0aCB0aGUg bmV4dAo+ICAgICBlcmFzZWJsb2NrLXNpemVkIHJlcXVlc3QuCj4gCj4gIDQuIG5hbmRfZG9fd3Jp dGVfb3BzKCkgc3BsaXRzIHRoZSBzZWNvbmQgcmVxdWVzdCB1cCBpbnRvIHBhZ2Utc2l6ZWQKPiAg ICAgcmVxdWVzdHMuICBUaGUgZmlyc3QgcGFnZSB3cml0ZSBjb250YWlucyAyMDQ4IGJ5dGVzIG9m IGRhdGEgYW5kIG5vCj4gICAgIE9PQiBkYXRhOyBzaW5jZSB0aGUgb29iYnVmIGZpZWxkIGluIHRo ZSBzdHJ1Y3QgbXRkX29vYl9vcHMgcGFzc2VkIGlzCj4gICAgIE5VTEwsIG9vYl9yZXF1aXJlZCBp cyBzZXQgdG8gMC4KPiAKPiAgNS4gVGhlIGFib3ZlIGNhdXNlcyB0aGUgZHJpdmVyIHRvIHJlY2Vp dmUgMjA0OCBieXRlcyBvZiBkYXRhIGZvciBhIHBhZ2UKPiAgICAgd3JpdGUgaW4gcmF3IG1vZGUs IHdoaWNoIHJlc3VsdHMgaW4gYW4gZXJyb3IgdGhhdCBwcm9wYWdhdGVzIGFsbCB0aGUKPiAgICAg d2F5IHVwIHRvIG10ZGNoYXJfd3JpdGVfaW9jdGwoKS4KClRoaXMgaXMgZGVmaW5pdGVseSBmYXIg ZnJvbSBhbiBleHBlY3RlZCBiZWhhdmlvci4gV3JpdGluZyBhIHBhZ2UKd2l0aG91dCBPT0IgaXMg Y29tcGxldGVseSBmaW5lLgoKPiAKPiBUaGUgbmFuZHNpbSBkcml2ZXIgcmV0dXJucyB0aGUgc2Ft ZSBlcnJvciBpZiB5b3UgcGFzcyB0aGUgZm9sbG93aW5nCj4gcmVxdWVzdCB0byB0aGUgTUVNV1JJ VEUgaW9jdGw6Cj4gCj4gICAgIHN0cnVjdCBtdGRfd3JpdGVfcmVxIHJlcSA9IHsKPiAgICAgICAg IC5zdGFydCA9IDIwNDgsCj4gICAgICAgICAubGVuID0gMjA0OCwKPiAgICAgICAgIC5vb2JsZW4g PSAwLAo+ICAgICAgICAgLnVzcl9kYXRhID0gMHgxMDAwMDAwMCwKPiAgICAgICAgIC51c3Jfb29i ID0gTlVMTCwKPiAgICAgICAgIC5tb2RlID0gTVREX09QU19SQVcsCj4gICAgIH07Cj4gCj4gc28g aXQgaXMgbm90IHRoZSBkcml2ZXIgdGhhdCBpcyBicm9rZW4gb3IgaW5zYW5lLCBpdCBpcyB0aGUg c3BsaXR0aW5nCj4gcHJvY2VzcyB0aGF0IG1heSBjYXVzZSB0aGUgTUVNV1JJVEUgaW9jdGwgdG8g cmV0dXJuIGRpZmZlcmVudCBlcnJvcgo+IGNvZGVzIHRoYW4gYmVmb3JlLgo+IAo+IEkgcGxheWVk IHdpdGggdGhlIGNvZGUgYSBiaXQgbW9yZSBhbmQgSSBmb3VuZCBhIGZpeCB3aGljaCBhZGRyZXNz ZXMgdGhpcwo+IGlzc3VlIHdpdGhvdXQgYnJlYWtpbmcgb3RoZXIgc2NlbmFyaW9zOiBzZXR0aW5n IG9vYmJ1ZiB0byB0aGUgc2FtZQo+IHBvaW50ZXIgZm9yIGV2ZXJ5IGxvb3AgaXRlcmF0aW9uIChp ZiBvb2JsZW4gaXMgMCwgbm8gT09CIGRhdGEgd2lsbCBiZQo+IHdyaXR0ZW4gYW55d2F5KS4KCllv dSBtZWFuIHRoYXQKCXsgLnVzZXJfb29iID0gTlVMTCwgLm9vYmxlbiA9IDAgfQpmYWlscywgd2hp bGUKCXsgLnVzZXJfb29iID0gcmFuZG9tLCAub29ibGVuID0gMCB9CndvcmtzPyBUaGlzIHNlZW1z IGEgbGl0dGxlIGJpdCBmcmFnaWxlLgoKQ291bGQgeW91IHRlbGwgdXMgdGhlIG9yaWdpbiBvZiB0 aGUgZXJyb3I/IEJlY2F1c2UgaW4KbmFuZF9kb193cml0ZV9vcHMoKSBpZiBvcHMtPm9vYmJ1ZiBp cyBwb3B1bGF0ZWQgdGhlbiBvb2JfcmVxdWlyZWQgaXMKc2V0IHRvIHRydWUgbm8gbWF0dGVyIHRo ZSB2YWx1ZSBzZXQgaW4gb29ibGVuLgoKUGx1cywgdGhlIGNvZGUgaW4gbXRkY2hhciBpcyBjbGVh cjogLm9vYmJ1ZiBpcyBzZXQgdG8gTlVMTCBpZiB0aGVyZSBhcmUKbm8gT09CcyBwcm92aWRlZCBi eSB0aGUgdXNlciBzbyBJIGJlbGlldmUgdGhpcyBpcyBhIHNpdHVhdGlvbiB0aGF0CnNob3VsZCBh bHJlYWR5IHdvcmsuIAoKPiBJIGFsc28gdGFja2xlZCB0aGUgcHJvYmxlbSBvZiBtaXNoYW5kbGlu ZyBsYXJnZSBub24tcGFnZS1hbGlnbmVkIHdyaXRlcwo+IGluIHYxIGFuZCBJIG1hbmFnZWQgdG8g Zml4IGl0IGJ5IHRyaW1taW5nIHRoZSBmaXJzdCBtdGRfd3JpdGVfb29iKCkgY2FsbAo+IHNvIHRo YXQgaXQgZW5kcyBvbiBhbiBlcmFzZWJsb2NrIGJvdW5kYXJ5LiAgVGhpcyBpbXBsaWNpdGx5IG1h a2VzCj4gc3Vic2VxdWVudCB3cml0ZXMgcGFnZS1hbGlnbmVkIGFuZCBzZWVtcyB0byBmaXggdGhl IHByb2JsZW0uCgpHcmVhdCEKCj4gCj4gRmluYWxseSwgSSByZXdvcmtlZCB0aGUgT09CIGxlbmd0 aCBhZGp1c3RtZW50IGNvZGUgdG8gYWRkcmVzcyBvdGhlcgo+IGNhc2VzIG9mIG1pc2hhbmRsaW5n IG5vbi1wYWdlLWFsaWduZWQgd3JpdGVzLgo+IAo+IEkgY291bGQgbm90IGZpbmQgYW55IG90aGVy IGNhc2VzIGluIHdoaWNoIHRoZSByZXZpc2VkIGNvZGUgYmVoYXZlcwo+IGRpZmZlcmVudGx5IHRo YW4gdGhlIG9yaWdpbmFsIG9uZS4gIEkgd2lsbCBzZW5kIHYyIHNvb24uCgpUaGFua3MgZm9yIGFs bCB3b3JrIG9uIHRoaXMgdG9waWMhCgpDaGVlcnMsCk1pcXXDqGwKCl9fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpMaW51eCBNVEQgZGlzY3Vzc2lv biBtYWlsaW5nIGxpc3QKaHR0cDovL2xpc3RzLmluZnJhZGVhZC5vcmcvbWFpbG1hbi9saXN0aW5m by9saW51eC1tdGQvCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 974A9C433FE for ; Fri, 26 Nov 2021 09:33:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1344081AbhKZJgk convert rfc822-to-8bit (ORCPT ); Fri, 26 Nov 2021 04:36:40 -0500 Received: from relay7-d.mail.gandi.net ([217.70.183.200]:43217 "EHLO relay7-d.mail.gandi.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1376303AbhKZJee (ORCPT ); Fri, 26 Nov 2021 04:34:34 -0500 Received: (Authenticated sender: miquel.raynal@bootlin.com) by relay7-d.mail.gandi.net (Postfix) with ESMTPSA id 4DE162000E; Fri, 26 Nov 2021 09:31:17 +0000 (UTC) Date: Fri, 26 Nov 2021 10:31:16 +0100 From: Miquel Raynal To: =?UTF-8?B?TWljaGHFgiBLxJlwaWXFhA==?= Cc: Richard Weinberger , Vignesh Raghavendra , Boris Brezillon , linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] mtdchar: prevent unbounded allocation in MEMWRITE ioctl Message-ID: <20211126103116.5bef6bc0@xps13> In-Reply-To: References: <20211025082104.8017-1-kernel@kempniu.pl> <20211122103122.424326a1@xps13> Organization: Bootlin X-Mailer: Claws Mail 3.17.7 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Michał, kernel@kempniu.pl wrote on Thu, 25 Nov 2021 13:08:16 +0100: > Hi Miquèl, > > TL;DR: thanks, your comment made me look closer and I found what seems > to be a feasible workaround that I will apply in v2 (along other fixes). > > > > Despite my efforts, the patch does _not_ retain absolute backward > > > compatibility and I do not know whether this is acceptable. > > > Specifically, multi-eraseblock-sized writes (requiring multiple loop > > > iterations to be processed) which succeeded with the original code _may_ > > > now return errors and/or write different contents to the device than the > > > original code, depending on the MTD mode of operation requested and on > > > whether the start offset is page-aligned. The documentation for struct > > > mtd_oob_ops warns about even multi-page write requests, but... > > > > > > Example: > > > > > > MTD device parameters: > > > > > > - mtd->writesize = 2048 > > > - mtd->erasesize = 131072 > > > - 64 bytes of raw OOB space per page > > > > > > struct mtd_write_req req = { > > > .start = 2048, > > > .len = 262144, > > > .ooblen = 64, > > > .usr_data = ..., > > > .usr_oob = ..., > > > .mode = MTD_OPS_RAW, > > > }; > > > > > > (This is a 128-page write with OOB data supplied for just one page.) > > > > > > Current mtdchar_write_ioctl() returns 0 for this request and writes > > > 128 pages of data and 1 page's worth of OOB data (64 bytes) to the > > > MTD device. > > > > > > Patched mtdchar_write_ioctl() may return an error because the > > > original request gets split into two chunks (): > > > > > > <131072, 64> > > > <131072, 0> > > > > > > and the second chunk with zero OOB data length may make the MTD > > > driver unhappy in raw mode (resulting in only the first 64 pages of > > > data and 1 page's worth of OOB data getting written to the MTD > > > device). > > > > Isn't this a driver issue instead? I mean, writing an eraseblock > > without providing any OOB data is completely fine, if the driver > > accepts 2 blocks + 1 page OOB but refuses 1 block + 1 page OOB and then > > 1 block, it's broken, no? Have you experienced such a situation in your > > testing? > > I may not have expressed myself clearly here, sorry - the example was > already getting a bit lengthy at that point... :) > > I tested the patch with nandsim, but I do not think it is that specific > driver that is broken. The catch is that when mtd_write_oob() is > called, nand_do_write_ops() splits multi-page requests into single-page > requests and what it passes to nand_write_page() depends on whether the > struct mtd_oob_ops it was invoked with has the oobbuf field set to NULL > or not. This is okay in itself, but when another request-splitting > "layer" is introduced by my patch, the ioctl may start returning > different result codes than it used to. > > Here is what happens with the unpatched code for the example above: > > 1. mtdchar_write_ioctl() gets called with the following request: > > struct mtd_write_req req = { > .start = 2048, > .len = 262144, > .ooblen = 64, > .usr_data = 0x10000000, > .usr_oob = 0x20000000, > .mode = MTD_OPS_RAW, > }; > > 2. The above request is passed through to mtd_write_oob() verbatim: > > struct mtd_oob_ops ops = { > .mode = MTD_OPS_RAW, > .len = 262144, > .ooblen = 64, > .datbuf = 0x10000000, > .oobbuf = 0x20000000, > }; > > 3. nand_do_write_ops() splits this request up into page-sized requests. > > a) For the first page, the initial 2048 bytes of data + 64 bytes of > OOB data are passed to nand_write_page(). > > b) For each subsequent page, a 2048-byte chunk of data + 64 bytes > of 0xff bytes are passed to nand_write_page(). > > Since the oobbuf field in the struct mtd_oob_ops passed is not NULL, > oob_required is set to 1 for all nand_write_page() calls. > > 4. The above causes the driver to receive 2112 bytes of data for each > page write, which results in the ioctl being successful. > > Here is what happens with the patched code: > > 1. mtdchar_write_ioctl() gets called with the same request as above. > > 2. The original request gets split into two eraseblock-sized > mtd_write_oob() calls: > > a) struct mtd_oob_ops ops = { > .mode = MTD_OPS_RAW, > .len = 131072, > .ooblen = 64, > .datbuf = 0x10000000, > .oobbuf = 0x20000000, > }; > > b) struct mtd_oob_ops ops = { > .mode = MTD_OPS_RAW, > .len = 131072, > .ooblen = 0, > .datbuf = 0x10020000, > .oobbuf = NULL, > }; > > (My code sets oobbuf to NULL if ooblen is 0.) > > 3. nand_do_write_ops() splits the first request up into page-sized > requests the same way as for the original code. It returns > successfully, so mtdchar_write_ioctl() proceeds with the next > eraseblock-sized request. > > 4. nand_do_write_ops() splits the second request up into page-sized > requests. The first page write contains 2048 bytes of data and no > OOB data; since the oobbuf field in the struct mtd_oob_ops passed is > NULL, oob_required is set to 0. > > 5. The above causes the driver to receive 2048 bytes of data for a page > write in raw mode, which results in an error that propagates all the > way up to mtdchar_write_ioctl(). This is definitely far from an expected behavior. Writing a page without OOB is completely fine. > > The nandsim driver returns the same error if you pass the following > request to the MEMWRITE ioctl: > > struct mtd_write_req req = { > .start = 2048, > .len = 2048, > .ooblen = 0, > .usr_data = 0x10000000, > .usr_oob = NULL, > .mode = MTD_OPS_RAW, > }; > > so it is not the driver that is broken or insane, it is the splitting > process that may cause the MEMWRITE ioctl to return different error > codes than before. > > I played with the code a bit more and I found a fix which addresses this > issue without breaking other scenarios: setting oobbuf to the same > pointer for every loop iteration (if ooblen is 0, no OOB data will be > written anyway). You mean that { .user_oob = NULL, .ooblen = 0 } fails, while { .user_oob = random, .ooblen = 0 } works? This seems a little bit fragile. Could you tell us the origin of the error? Because in nand_do_write_ops() if ops->oobbuf is populated then oob_required is set to true no matter the value set in ooblen. Plus, the code in mtdchar is clear: .oobbuf is set to NULL if there are no OOBs provided by the user so I believe this is a situation that should already work. > I also tackled the problem of mishandling large non-page-aligned writes > in v1 and I managed to fix it by trimming the first mtd_write_oob() call > so that it ends on an eraseblock boundary. This implicitly makes > subsequent writes page-aligned and seems to fix the problem. Great! > > Finally, I reworked the OOB length adjustment code to address other > cases of mishandling non-page-aligned writes. > > I could not find any other cases in which the revised code behaves > differently than the original one. I will send v2 soon. Thanks for all work on this topic! Cheers, Miquèl