linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Fix: ext4: add sanity check for inode inline write range
@ 2025-10-07 23:42 Ahmet Eray Karadag
  2025-10-08  0:47 ` Darrick J. Wong
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Ahmet Eray Karadag @ 2025-10-07 23:42 UTC (permalink / raw)
  To: tytso, adilger.kernel
  Cc: linux-ext4, linux-kernel, david.hunter.linux, skhan,
	Ahmet Eray Karadag, syzbot+f3185be57d7e8dda32b8,
	Albin Babu Varghese

Add a simple check in ext4_try_to_write_inline_data() to prevent
writes that extend past the inode's inline data area. The function
now returns -EINVAL if pos + len exceeds i_inline_size.

This avoids invalid inline write attempts and keeps the write path
consistent with the inode limits.

Reported-by: syzbot+f3185be57d7e8dda32b8@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=f3185be57d7e8dda32b8
Co-developed-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com> 
Signed-off-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
Signed-off-by: Ahmet Eray Karadag <eraykrdg1@gmail.com>
---
 fs/ext4/inline.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index 1b094a4f3866..13ba56e8e334 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -782,6 +782,16 @@ int ext4_write_inline_data_end(struct inode *inode, loff_t pos, unsigned len,
 	struct ext4_iloc iloc;
 	int ret = 0, ret2;
 
+	if ((pos + len) > EXT4_I(inode)->i_inline_size) {
+			ext4_warning_inode(inode,
+				"inline write beyond capacity (pos=%lld, len=%u, inline_size=%d)",
+				pos, len, EXT4_I(inode)->i_inline_size);
+		folio_unlock(folio);
+		folio_put(folio);
+		ret = -EINVAL;
+		goto out;
+	}
+
 	if (unlikely(copied < len) && !folio_test_uptodate(folio))
 		copied = 0;
 
@@ -838,8 +848,8 @@ int ext4_write_inline_data_end(struct inode *inode, loff_t pos, unsigned len,
 	 */
 	if (pos + len > inode->i_size && ext4_can_truncate(inode))
 		ext4_orphan_add(handle, inode);
-
-	ret2 = ext4_journal_stop(handle);
+	if (handle)
+		ret2 = ext4_journal_stop(handle);
 	if (!ret)
 		ret = ret2;
 	if (pos + len > inode->i_size) {
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix: ext4: add sanity check for inode inline write range
  2025-10-07 23:42 [PATCH] Fix: ext4: add sanity check for inode inline write range Ahmet Eray Karadag
@ 2025-10-08  0:47 ` Darrick J. Wong
  2025-10-08 12:34 ` Theodore Ts'o
  2025-10-10 11:31 ` kernel test robot
  2 siblings, 0 replies; 6+ messages in thread
From: Darrick J. Wong @ 2025-10-08  0:47 UTC (permalink / raw)
  To: Ahmet Eray Karadag
  Cc: tytso, adilger.kernel, linux-ext4, linux-kernel,
	david.hunter.linux, skhan, syzbot+f3185be57d7e8dda32b8,
	Albin Babu Varghese

On Wed, Oct 08, 2025 at 02:42:22AM +0300, Ahmet Eray Karadag wrote:
> Add a simple check in ext4_try_to_write_inline_data() to prevent
> writes that extend past the inode's inline data area. The function
> now returns -EINVAL if pos + len exceeds i_inline_size.
> 
> This avoids invalid inline write attempts and keeps the write path
> consistent with the inode limits.
> 
> Reported-by: syzbot+f3185be57d7e8dda32b8@syzkaller.appspotmail.com
> Link: https://syzkaller.appspot.com/bug?extid=f3185be57d7e8dda32b8
> Co-developed-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com> 
> Signed-off-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
> Signed-off-by: Ahmet Eray Karadag <eraykrdg1@gmail.com>
> ---
>  fs/ext4/inline.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> index 1b094a4f3866..13ba56e8e334 100644
> --- a/fs/ext4/inline.c
> +++ b/fs/ext4/inline.c
> @@ -782,6 +782,16 @@ int ext4_write_inline_data_end(struct inode *inode, loff_t pos, unsigned len,
>  	struct ext4_iloc iloc;
>  	int ret = 0, ret2;
>  
> +	if ((pos + len) > EXT4_I(inode)->i_inline_size) {
> +			ext4_warning_inode(inode,
> +				"inline write beyond capacity (pos=%lld, len=%u, inline_size=%d)",
> +				pos, len, EXT4_I(inode)->i_inline_size);
> +		folio_unlock(folio);
> +		folio_put(folio);
> +		ret = -EINVAL;
> +		goto out;

Shouldn't write_begin have converted the file to block format if the
range to be written exceeds the possible inlinedata size?

> +	}
> +
>  	if (unlikely(copied < len) && !folio_test_uptodate(folio))
>  		copied = 0;
>  
> @@ -838,8 +848,8 @@ int ext4_write_inline_data_end(struct inode *inode, loff_t pos, unsigned len,
>  	 */
>  	if (pos + len > inode->i_size && ext4_can_truncate(inode))
>  		ext4_orphan_add(handle, inode);
> -
> -	ret2 = ext4_journal_stop(handle);
> +	if (handle)
> +		ret2 = ext4_journal_stop(handle);

What is this??

--D

>  	if (!ret)
>  		ret = ret2;
>  	if (pos + len > inode->i_size) {
> -- 
> 2.43.0
> 
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix: ext4: add sanity check for inode inline write range
  2025-10-07 23:42 [PATCH] Fix: ext4: add sanity check for inode inline write range Ahmet Eray Karadag
  2025-10-08  0:47 ` Darrick J. Wong
@ 2025-10-08 12:34 ` Theodore Ts'o
  2025-10-08 13:40   ` Ahmet Eray Karadag
  2025-10-10 11:31 ` kernel test robot
  2 siblings, 1 reply; 6+ messages in thread
From: Theodore Ts'o @ 2025-10-08 12:34 UTC (permalink / raw)
  To: Ahmet Eray Karadag
  Cc: adilger.kernel, linux-ext4, linux-kernel, david.hunter.linux,
	skhan, syzbot+f3185be57d7e8dda32b8, Albin Babu Varghese

On Wed, Oct 08, 2025 at 02:42:22AM +0300, Ahmet Eray Karadag wrote:
> Add a simple check in ext4_try_to_write_inline_data() to prevent
> writes that extend past the inode's inline data area. The function
> now returns -EINVAL if pos + len exceeds i_inline_size.

The commit description doesn't match with what the patch does.  The
patch changes ext4_write_inline_data_end() and not
ext4_try_to_write_inline().  Ext4_try_to_write_inline_data() called
from ext4_write_begin(), and it does this:

	if (pos + len > ext4_get_max_inline_size(inode))
		return ext4_convert_inline_data_to_extent(mapping, inode);

So the write extends past the inline data area, in ext4_write_begin(),
it will have already been converted to a non-inline function.

The ext4_write_inline_data_end() function is called from
ext4_write_end(), so you need to figure out why we hadn't configured
the file away from inline data in ext4_write_begin().

> Reported-by: syzbot+f3185be57d7e8dda32b8@syzkaller.appspotmail.com
> Link: https://syzkaller.appspot.com/bug?extid=f3185be57d7e8dda32b8

Did you just randomly bash the code until the syzbot reproducer
stopped failing?  Please try to understand the code and the failure
much more deeply before attempting to change the code.

Cheers,

					- Ted

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix: ext4: add sanity check for inode inline write range
  2025-10-08 12:34 ` Theodore Ts'o
@ 2025-10-08 13:40   ` Ahmet Eray Karadag
  2025-10-09 14:21     ` David Hunter
  0 siblings, 1 reply; 6+ messages in thread
From: Ahmet Eray Karadag @ 2025-10-08 13:40 UTC (permalink / raw)
  To: Theodore Ts'o
  Cc: adilger.kernel, linux-ext4, linux-kernel, david.hunter.linux,
	skhan, syzbot+f3185be57d7e8dda32b8, Albin Babu Varghese

Hi Ted,

Thanks for the feedback.

You’re right — the commit description didn’t match the actual change,
and I appreciate you pointing that out.
After reading Darrick’s reply, I realized our patch was only
preventing the final crash rather than fixing the underlying issue.
Sorry for the confusion — I noticed that a bit too late after sending it out.

I’ll review things more carefully before sending any follow-ups.

Thanks again for the guidance.

Best regards,
Eray

Theodore Ts'o <tytso@mit.edu>, 8 Eki 2025 Çar, 15:34 tarihinde şunu yazdı:
>
> On Wed, Oct 08, 2025 at 02:42:22AM +0300, Ahmet Eray Karadag wrote:
> > Add a simple check in ext4_try_to_write_inline_data() to prevent
> > writes that extend past the inode's inline data area. The function
> > now returns -EINVAL if pos + len exceeds i_inline_size.
>
> The commit description doesn't match with what the patch does.  The
> patch changes ext4_write_inline_data_end() and not
> ext4_try_to_write_inline().  Ext4_try_to_write_inline_data() called
> from ext4_write_begin(), and it does this:
>
>         if (pos + len > ext4_get_max_inline_size(inode))
>                 return ext4_convert_inline_data_to_extent(mapping, inode);
>
> So the write extends past the inline data area, in ext4_write_begin(),
> it will have already been converted to a non-inline function.
>
> The ext4_write_inline_data_end() function is called from
> ext4_write_end(), so you need to figure out why we hadn't configured
> the file away from inline data in ext4_write_begin().
>
> > Reported-by: syzbot+f3185be57d7e8dda32b8@syzkaller.appspotmail.com
> > Link: https://syzkaller.appspot.com/bug?extid=f3185be57d7e8dda32b8
>
> Did you just randomly bash the code until the syzbot reproducer
> stopped failing?  Please try to understand the code and the failure
> much more deeply before attempting to change the code.
>
> Cheers,
>
>                                         - Ted

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix: ext4: add sanity check for inode inline write range
  2025-10-08 13:40   ` Ahmet Eray Karadag
@ 2025-10-09 14:21     ` David Hunter
  0 siblings, 0 replies; 6+ messages in thread
From: David Hunter @ 2025-10-09 14:21 UTC (permalink / raw)
  To: Ahmet Eray Karadag, Theodore Ts'o
  Cc: adilger.kernel, linux-ext4, linux-kernel, skhan,
	syzbot+f3185be57d7e8dda32b8, Albin Babu Varghese

On 10/8/25 09:40, Ahmet Eray Karadag wrote:
> I’ll review things more carefully before sending any follow-ups.

Hey,

I wanted to make some suggestions for you to do before sending a patch.
I recommend using tools like ftrace and the old classic printf
statements to verify that the code is doing what you are saying that the
code is doing.

If you are having trouble with the less intrusive tools, you can use
kgdb to verify your own code does what is expected, but do not use it as
a final verification because it is too intrusive and changes overall
kernel behavior.

Let me know if you are having any problems or need help with something.

Thanks,
David Hunter

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix: ext4: add sanity check for inode inline write range
  2025-10-07 23:42 [PATCH] Fix: ext4: add sanity check for inode inline write range Ahmet Eray Karadag
  2025-10-08  0:47 ` Darrick J. Wong
  2025-10-08 12:34 ` Theodore Ts'o
@ 2025-10-10 11:31 ` kernel test robot
  2 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2025-10-10 11:31 UTC (permalink / raw)
  To: Ahmet Eray Karadag, tytso, adilger.kernel
  Cc: oe-kbuild-all, linux-ext4, linux-kernel, david.hunter.linux,
	skhan, Ahmet Eray Karadag, syzbot+f3185be57d7e8dda32b8,
	Albin Babu Varghese

Hi Ahmet,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tytso-ext4/dev]
[also build test WARNING on linus/master v6.17 next-20251009]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ahmet-Eray-Karadag/Fix-ext4-add-sanity-check-for-inode-inline-write-range/20251010-013008
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
patch link:    https://lore.kernel.org/r/20251007234221.28643-2-eraykrdg1%40gmail.com
patch subject: [PATCH] Fix: ext4: add sanity check for inode inline write range
config: powerpc-randconfig-r073-20251010 (https://download.01.org/0day-ci/archive/20251010/202510101841.kobch6UW-lkp@intel.com/config)
compiler: powerpc-linux-gcc (GCC) 14.3.0

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510101841.kobch6UW-lkp@intel.com/

smatch warnings:
fs/ext4/inline.c:789 ext4_write_inline_data_end() warn: inconsistent indenting
fs/ext4/inline.c:854 ext4_write_inline_data_end() error: uninitialized symbol 'ret2'.

vim +789 fs/ext4/inline.c

   775	
   776	int ext4_write_inline_data_end(struct inode *inode, loff_t pos, unsigned len,
   777				       unsigned copied, struct folio *folio)
   778	{
   779		handle_t *handle = ext4_journal_current_handle();
   780		int no_expand;
   781		void *kaddr;
   782		struct ext4_iloc iloc;
   783		int ret = 0, ret2;
   784	
   785		if ((pos + len) > EXT4_I(inode)->i_inline_size) {
   786				ext4_warning_inode(inode,
   787					"inline write beyond capacity (pos=%lld, len=%u, inline_size=%d)",
   788					pos, len, EXT4_I(inode)->i_inline_size);
 > 789			folio_unlock(folio);
   790			folio_put(folio);
   791			ret = -EINVAL;
   792			goto out;
   793		}
   794	
   795		if (unlikely(copied < len) && !folio_test_uptodate(folio))
   796			copied = 0;
   797	
   798		if (likely(copied)) {
   799			ret = ext4_get_inode_loc(inode, &iloc);
   800			if (ret) {
   801				folio_unlock(folio);
   802				folio_put(folio);
   803				ext4_std_error(inode->i_sb, ret);
   804				goto out;
   805			}
   806			ext4_write_lock_xattr(inode, &no_expand);
   807			BUG_ON(!ext4_has_inline_data(inode));
   808	
   809			/*
   810			 * ei->i_inline_off may have changed since
   811			 * ext4_write_begin() called
   812			 * ext4_try_to_write_inline_data()
   813			 */
   814			(void) ext4_find_inline_data_nolock(inode);
   815	
   816			kaddr = kmap_local_folio(folio, 0);
   817			ext4_write_inline_data(inode, &iloc, kaddr, pos, copied);
   818			kunmap_local(kaddr);
   819			folio_mark_uptodate(folio);
   820			/* clear dirty flag so that writepages wouldn't work for us. */
   821			folio_clear_dirty(folio);
   822	
   823			ext4_write_unlock_xattr(inode, &no_expand);
   824			brelse(iloc.bh);
   825	
   826			/*
   827			 * It's important to update i_size while still holding folio
   828			 * lock: page writeout could otherwise come in and zero
   829			 * beyond i_size.
   830			 */
   831			ext4_update_inode_size(inode, pos + copied);
   832		}
   833		folio_unlock(folio);
   834		folio_put(folio);
   835	
   836		/*
   837		 * Don't mark the inode dirty under folio lock. First, it unnecessarily
   838		 * makes the holding time of folio lock longer. Second, it forces lock
   839		 * ordering of folio lock and transaction start for journaling
   840		 * filesystems.
   841		 */
   842		if (likely(copied))
   843			mark_inode_dirty(inode);
   844	out:
   845		/*
   846		 * If we didn't copy as much data as expected, we need to trim back
   847		 * size of xattr containing inline data.
   848		 */
   849		if (pos + len > inode->i_size && ext4_can_truncate(inode))
   850			ext4_orphan_add(handle, inode);
   851		if (handle)
   852			ret2 = ext4_journal_stop(handle);
   853		if (!ret)
 > 854			ret = ret2;
   855		if (pos + len > inode->i_size) {
   856			ext4_truncate_failed_write(inode);
   857			/*
   858			 * If truncate failed early the inode might still be
   859			 * on the orphan list; we need to make sure the inode
   860			 * is removed from the orphan list in that case.
   861			 */
   862			if (inode->i_nlink)
   863				ext4_orphan_del(NULL, inode);
   864		}
   865		return ret ? ret : copied;
   866	}
   867	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-10-10 11:31 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-07 23:42 [PATCH] Fix: ext4: add sanity check for inode inline write range Ahmet Eray Karadag
2025-10-08  0:47 ` Darrick J. Wong
2025-10-08 12:34 ` Theodore Ts'o
2025-10-08 13:40   ` Ahmet Eray Karadag
2025-10-09 14:21     ` David Hunter
2025-10-10 11:31 ` kernel test robot

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).