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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sat, 2 Mar 2019 01:54:16 +0900 From: "Tokunori Ikegami" <ikegami.t@...il.com> To: "'Boris Brezillon'" <boris.brezillon@...labora.com> Cc: "'Tokunori Ikegami'" <ikegami.t@...il.com>, <keescook@...omium.org>, <bbrezillon@...nel.org>, <richard@....at>, <linux-kernel@...r.kernel.org>, <marek.vasut@...il.com>, <linux-mtd@...ts.infradead.org>, <computersforpeace@...il.com>, <dwmw2@...radead.org>, "'liujian \(CE\)'" <liujian56@...wei.com>, <vigneshr@...com> Subject: RE: [PATCH v3] cfi: fix deadloop in cfi_cmdset_0002.c do_write_buffer Hi Boris-san, > -----Original Message----- > From: linux-mtd [mailto:linux-mtd-bounces@...ts.infradead.org] On Behalf > Of Boris Brezillon > Sent: Saturday, March 2, 2019 1:07 AM > To: Tokunori Ikegami > Cc: 'Tokunori Ikegami'; keescook@...omium.org; bbrezillon@...nel.org; > ikegami@...ied-telesis.co.jp; richard@....at; > linux-kernel@...r.kernel.org; marek.vasut@...il.com; > linux-mtd@...ts.infradead.org; computersforpeace@...il.com; > dwmw2@...radead.org; 'liujian (CE)'; vigneshr@...com > Subject: Re: [PATCH v3] cfi: fix deadloop in cfi_cmdset_0002.c > do_write_buffer > > Hi Ikegami, > > On Fri, 1 Mar 2019 23:51:16 +0900 > "Tokunori Ikegami" <ikegami_to@...oo.co.jp> wrote: > > > > Except this version no longer does what Vignesh suggested. See how you > > > no longer test if chip_good() is true if time_after() returns true. > So, > > > imagine the thread entering this function is preempted just after the > > > first chip_good() test, and resumed a few ms later. time_after() will > > > return true, but chip_good() might also return true, and you ignore > it. > > > > I think that the following 3 versions will be worked for time_after() > as a same result and follow the Vignesh-san suggestion. > > Let me show you how they are different: > > > > > 1. Original Vignesh-san suggestion > > > > if (chip_good(map, adr, datum)) { > > xip_enable(map, chip, adr); > > goto op_done; > > } > > --> thread preempted here > ==> chip_good() test becomes valid here > --> thread resumed here, but timeout has expired > > > > > if (time_after(jiffies, timeo)) { > > you enter this branch > > > /* Test chip_good() if TRUE incorrectly again so write > failure by time_after() can be avoided. */ > > if (chip_good(map, adr, datum)) { > > chip_good() returns true > > > xip_enable(map, chip, adr); > > goto op_done; > > } > > break; > > } > > > > 2. Liujian-san v3 patch > > > > /* Test chip_good() if FALSE correctly so write failure by > time_after() can be avoided. */ > > --> thread preempted here > ==> chip_good() test becomes valid here > --> thread resumed here, but timeout has expired > > > if (time_after(jiffies, timeo) && !chip_good(map, adr)) > > You do not enter this branch because the chip_good() test is done once > more in case of timeout. > > > break; > > > > if (chip_good(map, adr, datum)) { > > xip_enable(map, chip, adr); > > goto op_done; > > } > > > > 3. My idea > > > > /* Save current jiffies value before chip_good() to avoid write > failure by time_after() without testing chip_good() again. */ > > unsigned long now = jiffies; > > > > if (chip_good(map, adr, datum)) { > > xip_enable(map, chip, adr); > > goto op_done; > > } > > > > --> thread preempted here > ==> chip_good() test becomes valid here > --> thread resumed here, but timeout has expired > > > if (time_after(now, timeo)) > > You do enter this branch, and erroneously report a failure. I do not think that it is not entered here since the value timeo is compare with the saved value now before the chip_bood() by time_after(). > > > break; > > > > See now why your version is not correct? > > Regards, > > Boris > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/
Powered by blists - more mailing lists