From: Vanessa Page <Vebpe@outlook.com>
To: Yifei Liu <yifeliu@cs.stonybrook.edu>
Cc: "ezk@cs.stonybrook.edu" <ezk@cs.stonybrook.edu>,
"madkar@cs.stonybrook.edu" <madkar@cs.stonybrook.edu>,
David Woodhouse <dwmw2@infradead.org>,
Richard Weinberger <richard@nod.at>,
"Matthew Wilcox (Oracle)" <willy@infradead.org>,
Kyeong Yoo <kyeong.yoo@alliedtelesis.co.nz>,
"linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] jffs2: correct logic when creating a hole in jffs2_write_begin
Date: Mon, 25 Jul 2022 21:34:46 +0000 [thread overview]
Message-ID: <MN2PR17MB33751E0A2DFD72AA5E774DF5B8959@MN2PR17MB3375.namprd17.prod.outlook.com> (raw)
In-Reply-To: <MN2PR17MB337506A10F6AF87BBC514F97B8959@MN2PR17MB3375.namprd17.prod.outlook.com>
Koop😂😂😑😑😑😎😄😄😴😴👀👀💁🙈🙈🙈🙈💁👀👀😑💁😁😳😎😴😑👀😑😄🤣🤣😀😃😑😴😄🤥🤫😥😰🥵🥵😡😡🤬🤯😭😭😟😟🥳🤩🥸🥸🥸🙁😩🥺☹️😣😔😔😔😖😣😢😡🥶😰😰😥😥😶🌫️🤬🤬🤬🤯😶🌫️🥶😰😰🤫🤫😓🤯🤯🤯
Ooiggvklpoure🤬🤬🤬🤬🤬🤬🤬🤬🤬🤬🤯🤯🤯🤯🤯🤯🤯🤯🤯🤫😓🤬🤯🤯🤬🤬🙁😒😍😒😒😎🥸🤩🥳😟😖🥺😩😩🤩🥳🥳😟😖😣🙁😩☹️😟😫😤😭🤬🤩🤩🥸😎🙁😩😡😵😪😧🤥🤥😯😵🤮😬🤥😓🤗🤗😶😲🤤🤐🤒🤕😵💫😵💫😵💫😵💫😵💫😵💫😵💫😵💫😵💫😵💫😵💫🤑😼😽☠️💀💀👻👻👻👹👹👺👽👽💀💀👻💀💀💀☠️☠️☠️☠️
> On Jul 25, 2022, at 1:06 AM, Vanessa Page <Vebpe@outlook.com> wrote:
>
> 🥰🥰🥰🥰🥰🥰🥰🥰🥰😍👌😍👌😍😍😍😍😍☺️☺️☺️☺️☺️💕💕😚😚😚😚🥰😚😍😍😍😚😚😍☺️☺️☺️😍💕😚🥰🥰😍☺️☺️😚🥰😍☺️😍🥰😍☺️☺️💕🥰🥰😍☺️☺️😊
>
> 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/
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
next prev parent reply other threads:[~2022-07-25 21:35 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-25 4:58 [PATCH] jffs2: correct logic when creating a hole in jffs2_write_begin Yifei Liu
2022-07-25 5:03 ` Vanessa Page
2022-07-25 21:34 ` Vanessa Page [this message]
2022-07-25 22:07 ` Vanessa Page
2022-07-25 22:08 ` Vanessa Page
2022-07-25 22:08 ` Vanessa Page
2022-07-26 0:11 ` Vanessa Page
2022-08-03 15:53 Yifei Liu
2022-08-21 18:21 ` Joakim Tjernlund
2022-08-22 6:25 ` Richard Weinberger
2022-09-04 19:39 ` Yifei Liu
2022-09-21 21:30 ` Yifei Liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=MN2PR17MB33751E0A2DFD72AA5E774DF5B8959@MN2PR17MB3375.namprd17.prod.outlook.com \
--to=vebpe@outlook.com \
--cc=dwmw2@infradead.org \
--cc=ezk@cs.stonybrook.edu \
--cc=kyeong.yoo@alliedtelesis.co.nz \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=madkar@cs.stonybrook.edu \
--cc=richard@nod.at \
--cc=willy@infradead.org \
--cc=yifeliu@cs.stonybrook.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).