From mboxrd@z Thu Jan 1 00:00:00 1970 From: Richard Weinberger Date: Tue, 22 May 2018 13:23:13 +0200 Subject: [U-Boot] [PATCH v1] ubifs: avoid assert failed in ubifs.c In-Reply-To: References: <1524837084-7086-1-git-send-email-patrice.chotard@st.com> <2e8b68a0-931e-443a-9312-f921042cec23@gmail.com> Message-ID: <2409611.QNcF8E23Z4@blindfold> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Am Dienstag, 22. Mai 2018, 12:56:48 CEST schrieb Marek Vasut: > On 05/10/2018 10:57 PM, Marek Vasut wrote: > > On 04/27/2018 03:51 PM, Patrice Chotard wrote: > >> This patch solves assert failed displayed in the console during a boot. > >> The root cause is that the ubifs_inode is not already allocated when > >> ubifs_printdir and ubifs_finddir functions are called. > >> > >> Trace showing the issue: > >> feed 'boot.scr.uimg', ino 94, new f_pos 0x17b40ece > >> dent->ch.sqnum '7132', creat_sqnum 3886945402880 > >> UBIFS assert failed in ubifs_finddir at 436 > >> INODE ALLOCATION: creat_sqnum '7129' > >> Found U-Boot script /boot.scr.uimg > >> > >> Signed-off-by: Christophe Kerello > >> Signed-off-by: Patrice Chotard > > > > I ran into this too, but what I do not quite understand from the commit > > message is how hiding the error actually solves the problem that the > > assert points to. > > > > Why does the assert trigger in the first place ? > > > > What is the root cause of the issue that is being hidden by this patch? > > Bump? I had a look, the bug is deeper, ubifs_finddir() allocates a vfs inode manually and ignores UBIFS internals. ubifs_inode() will read beyond the allocated buffer. In best case the assert triggers because ->creat_sqnum is garbage, in worst case, U-Boot will just crash. AFAICT, the correct solution is to use ubifs_iget(). Then we can keep the assert and it will check for the right thing. Thanks, //richard