Skip to content

Commit 681dbb0

Browse files
Yifei-Liugregkh
authored andcommitted
jffs2: correct logic when creating a hole in jffs2_write_begin
[ Upstream commit 23892d3 ] 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> Signed-off-by: Richard Weinberger <richard@nod.at> Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent fc20625 commit 681dbb0

File tree

1 file changed

+7
-8
lines changed

1 file changed

+7
-8
lines changed

fs/jffs2/file.c

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -138,19 +138,18 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping,
138138
struct jffs2_inode_info *f = JFFS2_INODE_INFO(inode);
139139
struct jffs2_sb_info *c = JFFS2_SB_INFO(inode->i_sb);
140140
pgoff_t index = pos >> PAGE_SHIFT;
141-
uint32_t pageofs = index << PAGE_SHIFT;
142141
int ret = 0;
143142

144143
jffs2_dbg(1, "%s()\n", __func__);
145144

146-
if (pageofs > inode->i_size) {
147-
/* Make new hole frag from old EOF to new page */
145+
if (pos > inode->i_size) {
146+
/* Make new hole frag from old EOF to new position */
148147
struct jffs2_raw_inode ri;
149148
struct jffs2_full_dnode *fn;
150149
uint32_t alloc_len;
151150

152-
jffs2_dbg(1, "Writing new hole frag 0x%x-0x%x between current EOF and new page\n",
153-
(unsigned int)inode->i_size, pageofs);
151+
jffs2_dbg(1, "Writing new hole frag 0x%x-0x%x between current EOF and new position\n",
152+
(unsigned int)inode->i_size, (uint32_t)pos);
154153

155154
ret = jffs2_reserve_space(c, sizeof(ri), &alloc_len,
156155
ALLOC_NORMAL, JFFS2_SUMMARY_INODE_SIZE);
@@ -170,10 +169,10 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping,
170169
ri.mode = cpu_to_jemode(inode->i_mode);
171170
ri.uid = cpu_to_je16(i_uid_read(inode));
172171
ri.gid = cpu_to_je16(i_gid_read(inode));
173-
ri.isize = cpu_to_je32(max((uint32_t)inode->i_size, pageofs));
172+
ri.isize = cpu_to_je32((uint32_t)pos);
174173
ri.atime = ri.ctime = ri.mtime = cpu_to_je32(JFFS2_NOW());
175174
ri.offset = cpu_to_je32(inode->i_size);
176-
ri.dsize = cpu_to_je32(pageofs - inode->i_size);
175+
ri.dsize = cpu_to_je32((uint32_t)pos - inode->i_size);
177176
ri.csize = cpu_to_je32(0);
178177
ri.compr = JFFS2_COMPR_ZERO;
179178
ri.node_crc = cpu_to_je32(crc32(0, &ri, sizeof(ri)-8));
@@ -203,7 +202,7 @@ static int jffs2_write_begin(struct file *filp, struct address_space *mapping,
203202
goto out_err;
204203
}
205204
jffs2_complete_reservation(c);
206-
inode->i_size = pageofs;
205+
inode->i_size = pos;
207206
mutex_unlock(&f->sem);
208207
}
209208

0 commit comments

Comments
 (0)