From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.free-electrons.com ([62.4.15.54]) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1e43g2-0007FQ-Ic for linux-mtd@lists.infradead.org; Mon, 16 Oct 2017 11:35:25 +0000 Date: Mon, 16 Oct 2017 13:34:57 +0200 From: Boris Brezillon To: Roger Quadros Cc: Tony Lindgren , linux-omap , "linux-mtd@lists.infradead.org" , "Cooper Jr., Franklin" Subject: Re: gpmc-nand broken since v4.12 Message-ID: <20171016133457.74b2773f@bbrezillon> In-Reply-To: References: <7eae9266-558f-6578-66d7-7ab0eb659a81@ti.com> <20171013145033.5d1d9647@bbrezillon> <20171013222458.3f4c103e@bbrezillon> <20171013222907.16adf410@bbrezillon> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Hi Roger, On Mon, 16 Oct 2017 13:22:04 +0300 Roger Quadros wrote: > =EF=BB=BF > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/= Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki >=20 > On 13/10/17 23:29, Boris Brezillon wrote: > > On Fri, 13 Oct 2017 22:24:58 +0200 > > Boris Brezillon wrote: > > =20 > >> On Fri, 13 Oct 2017 14:50:33 +0200 > >> Boris Brezillon wrote: > >> =20 > >>> On Fri, 13 Oct 2017 14:57:29 +0300 > >>> Roger Quadros wrote: > >>> =20 > >>>> =EF=BB=BFHi Boris, > >>>> > >>>> NAND on gpmc-omap breaks for me while doing a unmount of a ubi volum= e since v4.12 > >>>> > >>>> Behaviour follows through in v4.13 and v4.14-rc as well. > >>>> > >>>> Do you have any idea about this? =20 > >> > >> More info on what has changed in 4.12: we no longer allocate the > >> nand_buffer struct + its internal buffer using a single big kmalloc > >> call, which means the nand_buffer struct is now likely to be allocated > >> in a small object slab (sizeof(nand_buffers) =3D 12). If you have a > >> use-after-free bug somewhere in the kernel, it might corrupt the =20 >=20 > Spot on. Problem starts from commit 4d5dea5725f3aa95b9b64e252540e435dd216= dbc > "mtd: nand: allocate aligned buffers if NAND_OWN_BUFFERS is unset" >=20 > >=20 > > I meant buffer-overflow, not use-after-free. =20 >=20 > Your analysis seems correct. >=20 > > =20 > >> nand_buffers object, which might explain the bug you see here. > >> =20 > >>> > >>> Can you try with this patch [1] applied and paste me the values print= ed > >>> just before the crash? > >>> > >>> [1]http://code.bulix.org/lc8xk0-209746 =20 >=20 > =3D=3D unmounting volume > [ 36.308792] nand: nand_write_subpage_hwecc:2575 ecc_calc =3D (null) = oob_poi =3D ed096800 > [ 36.317319] mtd_ooblayout_set_bytes:1330 dst =3D ed096802 src =3D (n= ull) > [ 36.324162] Unable to handle kernel NULL pointer dereference at virtua= l address 00000000 >=20 >=20 > Running the following patch > https://hastebin.com/ulogurutuz.php > shows >=20 > [ 37.260689] nand: nand_write_subpage_hwecc:2547:A ecc_calc =3D ed116e4= 0 oob_poi =3D ed117800 > [ 37.260772] nand: nand_write_subpage_hwecc:2556:A1:step 0, ecc_calc = =3D ed116e40 > [ 37.260779] nand: nand_write_subpage_hwecc:2562:A2:step 0, ecc_calc = =3D ed116e40 > [ 37.260834] nand: nand_write_subpage_hwecc:2556:A1:step 1, ecc_calc = =3D ed116e40 > [ 37.260840] nand: nand_write_subpage_hwecc:2567:A3-pre:step 1, ecc_cal= c =3D ed116e40 > [ 37.260846] omap_calculate_ecc_bch > [ 37.260856] nand: nand_write_subpage_hwecc:2570:A3-post:step 1, ecc_ca= lc =3D (null) > [ 37.260912] nand: nand_write_subpage_hwecc:2556:A1:step 2, ecc_calc = =3D (null) > [ 37.260918] nand: nand_write_subpage_hwecc:2562:A2:step 2, ecc_calc = =3D (null) > [ 37.260972] nand: nand_write_subpage_hwecc:2556:A1:step 3, ecc_calc = =3D (null) > [ 37.260978] nand: nand_write_subpage_hwecc:2562:A2:step 3, ecc_calc = =3D (null) > [ 37.260984] nand: nand_write_subpage_hwecc:2587:B ecc_calc =3D (null= ) oob_poi =3D ed117800 > [ 37.260991] mtd_ooblayout_set_bytes:1330 dst =3D ed117802 src =3D (n= ull) >=20 > which means omap_calculate_ecc_bch() it the culprit. >=20 > Looks like the function calculates and stores ECC for all sectors even if= we just want ECC > for just one sector (sub-page). Yes, looks like you find the root cause. >=20 > Is my understanding correct > - We should not be hooking the multi-sector ECC calculator omap_calculate= _ecc_bch() to ecc.calculate > - provide a new one sector ECC calculator function (for BCH4/8/16) for om= ap and hook it to ecc.calculate > OR > - override nand_read_subpage() and nand_write_subpage() using omap specif= ic implementation (for BCH4/8/16). Second solution sounds simpler because the ECC sector information is not passed to ecc->calculate() hook, which means you'd have to extract it from the ecc_calc pointer: (uintptr_t)(ecc_calc - chip->buffers->ecccalc) / chip->ecc.size and doing arithmetic on pointers is usually not a good idea. Anyway, I'd be fine with both solutions, so pick the one you prefer. Regards, Boris From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Brezillon Subject: Re: gpmc-nand broken since v4.12 Date: Mon, 16 Oct 2017 13:34:57 +0200 Message-ID: <20171016133457.74b2773f@bbrezillon> References: <7eae9266-558f-6578-66d7-7ab0eb659a81@ti.com> <20171013145033.5d1d9647@bbrezillon> <20171013222458.3f4c103e@bbrezillon> <20171013222907.16adf410@bbrezillon> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-mtd" Errors-To: linux-mtd-bounces+gldm-linux-mtd-36=gmane.org@lists.infradead.org To: Roger Quadros Cc: Tony Lindgren , linux-omap , "linux-mtd@lists.infradead.org" , "Cooper Jr., Franklin" List-Id: linux-omap@vger.kernel.org SGkgUm9nZXIsCgpPbiBNb24sIDE2IE9jdCAyMDE3IDEzOjIyOjA0ICswMzAwClJvZ2VyIFF1YWRy b3MgPHJvZ2VycUB0aS5jb20+IHdyb3RlOgoKPiDvu78KPiBUZXhhcyBJbnN0cnVtZW50cyBGaW5s YW5kIE95LCBQb3Jra2FsYW5rYXR1IDIyLCAwMDE4MCBIZWxzaW5raS4gWS10dW5udXMvQnVzaW5l c3MgSUQ6IDA2MTU1MjEtNC4gS290aXBhaWtrYS9Eb21pY2lsZTogSGVsc2lua2kKPiAKPiBPbiAx My8xMC8xNyAyMzoyOSwgQm9yaXMgQnJlemlsbG9uIHdyb3RlOgo+ID4gT24gRnJpLCAxMyBPY3Qg MjAxNyAyMjoyNDo1OCArMDIwMAo+ID4gQm9yaXMgQnJlemlsbG9uIDxib3Jpcy5icmV6aWxsb25A ZnJlZS1lbGVjdHJvbnMuY29tPiB3cm90ZToKPiA+ICAgCj4gPj4gT24gRnJpLCAxMyBPY3QgMjAx NyAxNDo1MDozMyArMDIwMAo+ID4+IEJvcmlzIEJyZXppbGxvbiA8Ym9yaXMuYnJlemlsbG9uQGZy ZWUtZWxlY3Ryb25zLmNvbT4gd3JvdGU6Cj4gPj4gIAo+ID4+PiBPbiBGcmksIDEzIE9jdCAyMDE3 IDE0OjU3OjI5ICswMzAwCj4gPj4+IFJvZ2VyIFF1YWRyb3MgPHJvZ2VycUB0aS5jb20+IHdyb3Rl Ogo+ID4+PiAgICAgCj4gPj4+PiDvu79IaSBCb3JpcywKPiA+Pj4+Cj4gPj4+PiBOQU5EIG9uIGdw bWMtb21hcCBicmVha3MgZm9yIG1lIHdoaWxlIGRvaW5nIGEgdW5tb3VudCBvZiBhIHViaSB2b2x1 bWUgc2luY2UgdjQuMTIKPiA+Pj4+Cj4gPj4+PiBCZWhhdmlvdXIgZm9sbG93cyB0aHJvdWdoIGlu IHY0LjEzIGFuZCB2NC4xNC1yYyBhcyB3ZWxsLgo+ID4+Pj4KPiA+Pj4+IERvIHlvdSBoYXZlIGFu eSBpZGVhIGFib3V0IHRoaXM/ICAgIAo+ID4+Cj4gPj4gTW9yZSBpbmZvIG9uIHdoYXQgaGFzIGNo YW5nZWQgaW4gNC4xMjogd2Ugbm8gbG9uZ2VyIGFsbG9jYXRlIHRoZQo+ID4+IG5hbmRfYnVmZmVy IHN0cnVjdCArIGl0cyBpbnRlcm5hbCBidWZmZXIgdXNpbmcgYSBzaW5nbGUgYmlnIGttYWxsb2MK PiA+PiBjYWxsLCB3aGljaCBtZWFucyB0aGUgbmFuZF9idWZmZXIgc3RydWN0IGlzIG5vdyBsaWtl bHkgdG8gYmUgYWxsb2NhdGVkCj4gPj4gaW4gYSBzbWFsbCBvYmplY3Qgc2xhYiAoc2l6ZW9mKG5h bmRfYnVmZmVycykgPSAxMikuIElmIHlvdSBoYXZlIGEKPiA+PiB1c2UtYWZ0ZXItZnJlZSBidWcg c29tZXdoZXJlIGluIHRoZSBrZXJuZWwsIGl0IG1pZ2h0IGNvcnJ1cHQgdGhlICAKPiAKPiBTcG90 IG9uLiBQcm9ibGVtIHN0YXJ0cyBmcm9tIGNvbW1pdCA0ZDVkZWE1NzI1ZjNhYTk1YjliNjRlMjUy NTQwZTQzNWRkMjE2ZGJjCj4gIm10ZDogbmFuZDogYWxsb2NhdGUgYWxpZ25lZCBidWZmZXJzIGlm IE5BTkRfT1dOX0JVRkZFUlMgaXMgdW5zZXQiCj4gCj4gPiAKPiA+IEkgbWVhbnQgYnVmZmVyLW92 ZXJmbG93LCBub3QgdXNlLWFmdGVyLWZyZWUuICAKPiAKPiBZb3VyIGFuYWx5c2lzIHNlZW1zIGNv cnJlY3QuCj4gCj4gPiAgIAo+ID4+IG5hbmRfYnVmZmVycyBvYmplY3QsIHdoaWNoIG1pZ2h0IGV4 cGxhaW4gdGhlIGJ1ZyB5b3Ugc2VlIGhlcmUuCj4gPj4gIAo+ID4+Pgo+ID4+PiBDYW4geW91IHRy eSB3aXRoIHRoaXMgcGF0Y2ggWzFdIGFwcGxpZWQgYW5kIHBhc3RlIG1lIHRoZSB2YWx1ZXMgcHJp bnRlZAo+ID4+PiBqdXN0IGJlZm9yZSB0aGUgY3Jhc2g/Cj4gPj4+Cj4gPj4+IFsxXWh0dHA6Ly9j b2RlLmJ1bGl4Lm9yZy9sYzh4azAtMjA5NzQ2ICAKPiAKPiA9PSB1bm1vdW50aW5nIHZvbHVtZQo+ IFsgICAzNi4zMDg3OTJdIG5hbmQ6IG5hbmRfd3JpdGVfc3VicGFnZV9od2VjYzoyNTc1IGVjY19j YWxjID0gICAobnVsbCkgb29iX3BvaSA9IGVkMDk2ODAwCj4gWyAgIDM2LjMxNzMxOV0gbXRkX29v YmxheW91dF9zZXRfYnl0ZXM6MTMzMCBkc3QgPSBlZDA5NjgwMiBzcmMgPSAgIChudWxsKQo+IFsg ICAzNi4zMjQxNjJdIFVuYWJsZSB0byBoYW5kbGUga2VybmVsIE5VTEwgcG9pbnRlciBkZXJlZmVy ZW5jZSBhdCB2aXJ0dWFsIGFkZHJlc3MgMDAwMDAwMDAKPiAKPiAKPiBSdW5uaW5nIHRoZSBmb2xs b3dpbmcgcGF0Y2gKPiBodHRwczovL2hhc3RlYmluLmNvbS91bG9ndXJ1dHV6LnBocAo+IHNob3dz Cj4gCj4gWyAgIDM3LjI2MDY4OV0gbmFuZDogbmFuZF93cml0ZV9zdWJwYWdlX2h3ZWNjOjI1NDc6 QSBlY2NfY2FsYyA9IGVkMTE2ZTQwIG9vYl9wb2kgPSBlZDExNzgwMAo+IFsgICAzNy4yNjA3NzJd IG5hbmQ6IG5hbmRfd3JpdGVfc3VicGFnZV9od2VjYzoyNTU2OkExOnN0ZXAgMCwgZWNjX2NhbGMg PSBlZDExNmU0MAo+IFsgICAzNy4yNjA3NzldIG5hbmQ6IG5hbmRfd3JpdGVfc3VicGFnZV9od2Vj YzoyNTYyOkEyOnN0ZXAgMCwgZWNjX2NhbGMgPSBlZDExNmU0MAo+IFsgICAzNy4yNjA4MzRdIG5h bmQ6IG5hbmRfd3JpdGVfc3VicGFnZV9od2VjYzoyNTU2OkExOnN0ZXAgMSwgZWNjX2NhbGMgPSBl ZDExNmU0MAo+IFsgICAzNy4yNjA4NDBdIG5hbmQ6IG5hbmRfd3JpdGVfc3VicGFnZV9od2VjYzoy NTY3OkEzLXByZTpzdGVwIDEsIGVjY19jYWxjID0gZWQxMTZlNDAKPiBbICAgMzcuMjYwODQ2XSBv bWFwX2NhbGN1bGF0ZV9lY2NfYmNoCj4gWyAgIDM3LjI2MDg1Nl0gbmFuZDogbmFuZF93cml0ZV9z dWJwYWdlX2h3ZWNjOjI1NzA6QTMtcG9zdDpzdGVwIDEsIGVjY19jYWxjID0gICAobnVsbCkKPiBb ICAgMzcuMjYwOTEyXSBuYW5kOiBuYW5kX3dyaXRlX3N1YnBhZ2VfaHdlY2M6MjU1NjpBMTpzdGVw IDIsIGVjY19jYWxjID0gICAobnVsbCkKPiBbICAgMzcuMjYwOTE4XSBuYW5kOiBuYW5kX3dyaXRl X3N1YnBhZ2VfaHdlY2M6MjU2MjpBMjpzdGVwIDIsIGVjY19jYWxjID0gICAobnVsbCkKPiBbICAg MzcuMjYwOTcyXSBuYW5kOiBuYW5kX3dyaXRlX3N1YnBhZ2VfaHdlY2M6MjU1NjpBMTpzdGVwIDMs IGVjY19jYWxjID0gICAobnVsbCkKPiBbICAgMzcuMjYwOTc4XSBuYW5kOiBuYW5kX3dyaXRlX3N1 YnBhZ2VfaHdlY2M6MjU2MjpBMjpzdGVwIDMsIGVjY19jYWxjID0gICAobnVsbCkKPiBbICAgMzcu MjYwOTg0XSBuYW5kOiBuYW5kX3dyaXRlX3N1YnBhZ2VfaHdlY2M6MjU4NzpCIGVjY19jYWxjID0g ICAobnVsbCkgb29iX3BvaSA9IGVkMTE3ODAwCj4gWyAgIDM3LjI2MDk5MV0gbXRkX29vYmxheW91 dF9zZXRfYnl0ZXM6MTMzMCBkc3QgPSBlZDExNzgwMiBzcmMgPSAgIChudWxsKQo+IAo+IHdoaWNo IG1lYW5zIG9tYXBfY2FsY3VsYXRlX2VjY19iY2goKSBpdCB0aGUgY3VscHJpdC4KPiAKPiBMb29r cyBsaWtlIHRoZSBmdW5jdGlvbiBjYWxjdWxhdGVzIGFuZCBzdG9yZXMgRUNDIGZvciBhbGwgc2Vj dG9ycyBldmVuIGlmIHdlIGp1c3Qgd2FudCBFQ0MKPiBmb3IganVzdCBvbmUgc2VjdG9yIChzdWIt cGFnZSkuCgpZZXMsIGxvb2tzIGxpa2UgeW91IGZpbmQgdGhlIHJvb3QgY2F1c2UuCgo+IAo+IElz IG15IHVuZGVyc3RhbmRpbmcgY29ycmVjdAo+IC0gV2Ugc2hvdWxkIG5vdCBiZSBob29raW5nIHRo ZSBtdWx0aS1zZWN0b3IgRUNDIGNhbGN1bGF0b3Igb21hcF9jYWxjdWxhdGVfZWNjX2JjaCgpIHRv IGVjYy5jYWxjdWxhdGUKPiAtIHByb3ZpZGUgYSBuZXcgb25lIHNlY3RvciBFQ0MgY2FsY3VsYXRv ciBmdW5jdGlvbiAoZm9yIEJDSDQvOC8xNikgZm9yIG9tYXAgYW5kIGhvb2sgaXQgdG8gZWNjLmNh bGN1bGF0ZQo+IE9SCj4gLSBvdmVycmlkZSBuYW5kX3JlYWRfc3VicGFnZSgpIGFuZCBuYW5kX3dy aXRlX3N1YnBhZ2UoKSB1c2luZyBvbWFwIHNwZWNpZmljIGltcGxlbWVudGF0aW9uIChmb3IgQkNI NC84LzE2KS4KClNlY29uZCBzb2x1dGlvbiBzb3VuZHMgc2ltcGxlciBiZWNhdXNlIHRoZSBFQ0Mg c2VjdG9yIGluZm9ybWF0aW9uIGlzCm5vdCBwYXNzZWQgdG8gZWNjLT5jYWxjdWxhdGUoKSBob29r LCB3aGljaCBtZWFucyB5b3UnZCBoYXZlIHRvIGV4dHJhY3QKaXQgZnJvbSB0aGUgZWNjX2NhbGMg cG9pbnRlcjoKCgkodWludHB0cl90KShlY2NfY2FsYyAtIGNoaXAtPmJ1ZmZlcnMtPmVjY2NhbGMp IC8gY2hpcC0+ZWNjLnNpemUKCmFuZCBkb2luZyBhcml0aG1ldGljIG9uIHBvaW50ZXJzIGlzIHVz dWFsbHkgbm90IGEgZ29vZCBpZGVhLgoKQW55d2F5LCBJJ2QgYmUgZmluZSB3aXRoIGJvdGggc29s dXRpb25zLCBzbyBwaWNrIHRoZSBvbmUgeW91IHByZWZlci4KClJlZ2FyZHMsCgpCb3JpcwoKX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCkxpbnV4 IE1URCBkaXNjdXNzaW9uIG1haWxpbmcgbGlzdApodHRwOi8vbGlzdHMuaW5mcmFkZWFkLm9yZy9t YWlsbWFuL2xpc3RpbmZvL2xpbnV4LW10ZC8K