Messages in this thread | | | From | Vanessa Page <> | Subject | Re: [PATCH] jffs2: correct logic when creating a hole in jffs2_write_begin | Date | Mon, 25 Jul 2022 05:03:59 +0000 |
| |
🥰🥰🥰🥰🥰🥰🥰🥰🥰😍👌😍👌😍😍😍😍😍☺️☺️☺️☺️☺️💕💕😚😚😚😚🥰😚😍😍😍😚😚😍☺️☺️☺️😍💕😚🥰🥰😍☺️☺️😚🥰😍☺️😍🥰😍☺️☺️💕🥰🥰😍☺️☺️😊
Sent from my iPhon✌️
> On Jul 25, 2022, at 1:01 AM, Yifei Liu <yifeliu@cs.stonybrook.edu> wrote: > > Bug description and fix: > > 1. Write data to a file, say all 1s from offset 0 to 16. > > 2. Truncate the file to a smaller size, say 8 bytes. > > 3. Write new bytes (say 2s) from an offset past the original size of the > file, say at offset 20, for 4 bytes. This is supposed to create a "hole" > in the file, meaning that the bytes from offset 8 (where it was truncated > above) up to the new write at offset 20, should all be 0s (zeros). > > 4. flush all caches using "echo 3 > /proc/sys/vm/drop_caches" (or unmount > and remount) the f/s. > > 5. Check the content of the file. It is wrong. The 1s that used to be > between bytes 9 and 16, before the truncation, have REAPPEARED (they should > be 0s). > > We wrote a script and helper C program to reproduce the bug > (reproduce_jffs2_write_begin_issue.sh, write_file.c, and Makefile). We can > make them available to anyone. > > The above example is shown when writing a small file within the same first > page. But the bug happens for larger files, as long as steps 1, 2, and 3 > above all happen within the same page. > > The problem was traced to the jffs2_write_begin code, where it goes into an > 'if' statement intended to handle writes past the current EOF (i.e., writes > that may create a hole). The code computes a 'pageofs' that is the floor > of the write position (pos), aligned to the page size boundary. In other > words, 'pageofs' will never be larger than 'pos'. The code then sets the > internal jffs2_raw_inode->isize to the size of max(current inode size, > pageofs) but that is wrong: the new file size should be the 'pos', which is > larger than both the current inode size and pageofs. > > Similarly, the code incorrectly sets the internal jffs2_raw_inode->dsize to > the difference between the pageofs minus current inode size; instead it > should be the current pos minus the current inode size. Finally, > inode->i_size was also set incorrectly. > > The patch below fixes this bug. The bug was discovered using a new tool > for finding f/s bugs using model checking, called MCFS (Model Checking File > Systems). > > Signed-off-by: Yifei Liu <yifeliu@cs.stonybrook.edu> > Signed-off-by: Erez Zadok <ezk@cs.stonybrook.edu> > Signed-off-by: Manish Adkar <madkar@cs.stonybrook.edu> > --- > fs/jffs2/file.c | 15 +++++++-------- > 1 file changed, 7 insertions(+), 8 deletions(-) > > diff --git a/fs/jffs2/file.c b/fs/jffs2/file.c > index ba86acbe12d3..0479096b96e4 100644 > --- a/fs/jffs2/file.c > +++ b/fs/jffs2/file.c > @@ -137,19 +137,18 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping, > struct jffs2_inode_info *f = JFFS2_INODE_INFO(inode); > struct jffs2_sb_info *c = JFFS2_SB_INFO(inode->i_sb); > pgoff_t index = pos >> PAGE_SHIFT; > - uint32_t pageofs = index << PAGE_SHIFT; > int ret = 0; > > jffs2_dbg(1, "%s()\n", __func__); > > - if (pageofs > inode->i_size) { > - /* Make new hole frag from old EOF to new page */ > + if (pos > inode->i_size) { > + /* Make new hole frag from old EOF to new position */ > struct jffs2_raw_inode ri; > struct jffs2_full_dnode *fn; > uint32_t alloc_len; > > - jffs2_dbg(1, "Writing new hole frag 0x%x-0x%x between current EOF and new page\n", > - (unsigned int)inode->i_size, pageofs); > + jffs2_dbg(1, "Writing new hole frag 0x%x-0x%x between current EOF and new position\n", > + (unsigned int)inode->i_size, (uint32_t)pos); > > ret = jffs2_reserve_space(c, sizeof(ri), &alloc_len, > ALLOC_NORMAL, JFFS2_SUMMARY_INODE_SIZE); > @@ -169,10 +168,10 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping, > ri.mode = cpu_to_jemode(inode->i_mode); > ri.uid = cpu_to_je16(i_uid_read(inode)); > ri.gid = cpu_to_je16(i_gid_read(inode)); > - ri.isize = cpu_to_je32(max((uint32_t)inode->i_size, pageofs)); > + ri.isize = cpu_to_je32((uint32_t)pos); > ri.atime = ri.ctime = ri.mtime = cpu_to_je32(JFFS2_NOW()); > ri.offset = cpu_to_je32(inode->i_size); > - ri.dsize = cpu_to_je32(pageofs - inode->i_size); > + ri.dsize = cpu_to_je32((uint32_t)pos - inode->i_size); > ri.csize = cpu_to_je32(0); > ri.compr = JFFS2_COMPR_ZERO; > ri.node_crc = cpu_to_je32(crc32(0, &ri, sizeof(ri)-8)); > @@ -202,7 +201,7 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping, > goto out_err; > } > jffs2_complete_reservation(c); > - inode->i_size = pageofs; > + inode->i_size = pos; > mutex_unlock(&f->sem); > } > > -- > 2.25.1 > > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/
| |